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

Improve API code organization #355

Merged
merged 4 commits into from
Sep 2, 2024
Merged

Improve API code organization #355

merged 4 commits into from
Sep 2, 2024

Conversation

detsch
Copy link
Member

@detsch detsch commented Aug 26, 2024

Add some local changes in order to better organize the api.cc code. Key objective is to simplify CheckIn* methods, moving code to separate methods when appropriate, and also reducing code duplication. There is still work to be done in that direction, but the current set of changes already represent an improvement.


@mike-sul I still need to give it some additional testing, but it should be ready for review.

@detsch detsch requested a review from mike-sul August 26, 2024 23:02
@mike-sul
Copy link
Contributor

This change requires lots of testing, especially for the offline update.

src/api.cc Outdated Show resolved Hide resolved
@detsch detsch force-pushed the detsch-improve-api-code branch 2 times, most recently from 4d125c2 to b17ba80 Compare August 27, 2024 20:56
src/api.cc Show resolved Hide resolved
@detsch detsch force-pushed the detsch-improve-api-code branch from b17ba80 to 619feff Compare August 28, 2024 18:12
Copy link
Contributor

@mike-sul mike-sul left a comment

Choose a reason for hiding this comment

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

LGTM. Let's do extensive e2e testing and merge it then.

detsch added 4 commits August 30, 2024 13:55
Main change is the addition of updateMeta function, to reduce code
duplication between CheckIn and CheckInLocal, specially related to error
handling.

Additional methods added: checkInFailure, getBundleMeta, getRepoSource.

Signed-off-by: Andre Detsch <[email protected]>
Simplify one boolean expression, and adjust style of code when returning
a CheckInResult instance.

Signed-off-by: Andre Detsch <[email protected]>
getAvailableTargets is always called with `just_latest = false`. Remove
the parameter and set the behavior to always return all available
targets from the allowed_targets list.

Signed-off-by: Andre Detsch <[email protected]>
@detsch detsch force-pushed the detsch-improve-api-code branch from 619feff to 7e3a249 Compare September 2, 2024 13:56
@detsch
Copy link
Member Author

detsch commented Sep 2, 2024

Did additional e2e testing, no problems found. Merging.

@detsch detsch merged commit 13e5321 into master Sep 2, 2024
4 checks passed
@detsch detsch deleted the detsch-improve-api-code branch September 2, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants