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

Update Contributing.md #57658

Closed
wants to merge 1 commit into from
Closed

Update Contributing.md #57658

wants to merge 1 commit into from

Conversation

cortices
Copy link
Contributor

@cortices cortices commented Jul 8, 2020

Indicate that revision line should come before head line as enforced by brew style. Also suggest to run brew style again to double-check.

Indicate that `revision` line should come before `head` line as enforced by `brew style`. Also suggest to run brew style again to double-check.
@@ -42,8 +42,8 @@ brew style foo
```

After testing, if you think it is needed to force the corresponding bottles to be
rebuilt and redistributed, add a line of the form `revision 1` to the formula,
or add 1 to the revision number already present.
rebuilt and redistributed, add a line of the form `revision 1` to the formula above the `head` line,
Copy link
Member

Choose a reason for hiding this comment

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

Not every formula has a head line so this is a bit confusing 🤔. Maybe we could recommend brew style --fix instead which will fix this up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps

Suggested change
rebuilt and redistributed, add a line of the form `revision 1` to the formula above the `head` line,
rebuilt and redistributed, add a line of the form `revision 1` to the formula,
or add 1 to the revision number already present. Run `brew style foo` again, and if desired include the `--fix` flag to automatically correct the formula to standard style.

Alternatively, is it important that the revision tag be suggested only after install/testing? Could it not be included in the earlier section around L31:

If you are already well versed in the use of `git`, then you can find the local
copy of the `homebrew-core` repository in this directory
(`$(brew --prefix)/Homebrew/Library/Taps/homebrew/homebrew-core/`).
Modify the formula there, leaving the section `bottle do ... end` unchanged, and if you think it is needed to force the corresponding bottles to be rebuilt and redistributed, add a line of the form `revision 1` to the formula,
 or add 1 to the revision number already present.
Prepare a pull request as you usually do.  Before submitting your pull request, be sure to test it
with these commands:

Copy link
Member

Choose a reason for hiding this comment

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

Yeh, it feels like it makes more sense earlier. I like the idea of just recommending brew style --fix before committing 👍

@stale
Copy link

stale bot commented Jul 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale No recent activity label Jul 29, 2020
@stale stale bot closed this Aug 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants