Skip to content

refactor: Simplify UpdateApplication mechanism, get rid of the SKIP func #9

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

Merged
merged 1 commit into from
Jan 25, 2022

Conversation

patrykwozinski
Copy link
Contributor

@patrykwozinski patrykwozinski commented Jan 18, 2022

In general, this small refactor is about getting rid of returning 3 params including the skip boolean value.

I have realized that if we're returning an empty list of ChangeEntry - that means there are no changes, and it's equivalent of the skip functionality.

image

@patrykwozinski patrykwozinski changed the title refactor: Cleanup UpdateApplication mechanism, get rid of the SKIP func refactor: Simplify UpdateApplication mechanism, get rid of the SKIP func Jan 18, 2022
Comment on lines -102 to -104
//TODO: Review how to do when skip
} else if skip {
return nil, nil
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the goal of these changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks great :D

@patrykwozinski patrykwozinski marked this pull request as ready for review January 18, 2022 14:07
@patrykwozinski patrykwozinski merged commit 6ff839f into develop Jan 25, 2022
@patrykwozinski patrykwozinski deleted the cleanup-commit-mechanism branch January 25, 2022 11:47
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.

4 participants