-
Notifications
You must be signed in to change notification settings - Fork 56
refactor(gitlab): simplify allow_publish function #41
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
refactor(gitlab): simplify allow_publish function #41
Conversation
This PR simplifies the allow_publish by keeping the same functionality. Removes unnecessary over-nesting and minor code duplication. Makes minor white-space arrangements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not the decider of the style, but I would always keep braces in ifs, even in a single line, more maintanable option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have been using this style in other projects. However I am also okay to bring it back if you both prefer that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool contribution, automated testing (where I'm working right now) would make this safer :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this simplification!
This PR simplifies the allow_publish by keeping the same functionality.
Removes unnecessary over-nesting and minor code duplication.
Makes minor white-space arrangements.