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

Fixes #26950 - PR review checklist #305

Merged
merged 2 commits into from Aug 27, 2019

Conversation

@mbacovsky
Copy link
Member

mbacovsky commented Jun 4, 2019

Hopefully this list makes the reviews easier.

Please comment what is not clear or what is missing.

@mbacovsky

This comment has been minimized.

Copy link
Member Author

mbacovsky commented Jun 4, 2019

@ofedoren, @shiramax, @rahulbajaj0509 please check the list.

Copy link
Member

ofedoren left a comment

@mbacovsky, good list. I've got only two minor things to add:

  • The code is readable and understandable
  • The code style is consistent with already existing
@mbacovsky

This comment has been minimized.

Copy link
Member Author

mbacovsky commented Jun 6, 2019

@ofedoren, good ones, updated.

@ofedoren

This comment has been minimized.

Copy link
Member

ofedoren commented Aug 27, 2019

@mbacovsky, LGTM and since nobody added nothing, I'm merging it.

In a case of new additions we can simply add to those within a new PR :)

@ofedoren ofedoren merged commit 7d78928 into theforeman:master Aug 27, 2019
2 checks passed
2 checks passed
Redmine issues Valid issues
Details
hammer Build finished. 2361 tests run, 0 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.