Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#754 | Implementer friendliness for avni #778

Merged
merged 8 commits into from
Sep 20, 2024

Conversation

shraddha761
Copy link
Contributor

@shraddha761 shraddha761 commented Aug 22, 2024

Shows the difference between two zip file which contains json form.

How it Works

  • send post request on url -> api/compare-metadata.
  • Differences of two zip file is shown is added, removed, modified and also with noModification.

Copy link
Contributor

@himeshr himeshr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. In General, we would like the MetadataDiffService to be further modularized.

Ideally break it into following mini services:

  • MetadataBundleAndFileHandler : To take care of unzip, read files only
  • MetadataDiffChecker: To identify differences
  • MetadataDiffOutputGenerator: to create diff objects of various types
  • etc.. additional ones as need

2. Also, breakdown large methods into smaller methods with appropriate names to make the code more readable.

3. Replace all string literals with String constants

Copy link
Contributor

@himeshr himeshr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There exists a cyclic dependency between MetadataDiffChecker and MetadataDiffOutputGenerator. We should avoid these in-general

@himeshr himeshr merged commit 40f8dc3 into avniproject:master Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Analysis
Development

Successfully merging this pull request may close these issues.

2 participants