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

various formulae: style fixes #80302

Merged
merged 32 commits into from Jul 6, 2021
Merged

Conversation

nandahkrishna
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

This PR is required for Homebrew/brew#11626. Some changes were made with brew style --fix, but others had to be done manually (where autocorrect wouldn't work properly).

Formulae where the changes weren't only style changes (but straightforward), I tried to build and test the formula locally. If we'd rather run CI here, I'll remove the CI-syntax-only label.

Formula/blaze.rb Outdated Show resolved Hide resolved
@carlocab
Copy link
Member

carlocab commented Jun 30, 2021

I'd rather run CI on this. Not all of these changes are obviously NFC (no functional change).

Though maybe we can wait till the CI queue is less backed up? Unless this is urgent, anyway.

@nandahkrishna
Copy link
Member Author

Not urgent and the CI queue being backed up was my main concern. I'll mark this as "do not merge" until we get a chance to run CI.

@nandahkrishna nandahkrishna added do not merge in progress Stale bot should stay away and removed CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. do not merge in progress Stale bot should stay away labels Jun 30, 2021
Formula/blaze.rb Outdated Show resolved Hide resolved
Formula/cidrmerge.rb Outdated Show resolved Hide resolved
Formula/fdroidserver.rb Outdated Show resolved Hide resolved
Formula/fn.rb Outdated Show resolved Hide resolved
Formula/httpd.rb Outdated Show resolved Hide resolved
Formula/ocrmypdf.rb Outdated Show resolved Hide resolved
Formula/pmdmini.rb Outdated Show resolved Hide resolved
Formula/speech-tools.rb Outdated Show resolved Hide resolved
Formula/sqlcipher.rb Outdated Show resolved Hide resolved
Formula/volatility.rb Outdated Show resolved Hide resolved
Formula/weboob.rb Outdated Show resolved Hide resolved
@carlocab
Copy link
Member

carlocab commented Jul 1, 2021

Most of these are improvements but some of them are... not.

@nandahkrishna
Copy link
Member Author

I'll rebase this once the Pillow-related PRs are merged. I'll also manually edit the ones that autocorrect spoilt, to the best of my ability 😅.

@nandahkrishna
Copy link
Member Author

ARM:

Error: 4 failed steps!
brew install --build-from-source flow
brew install --build-from-source hashlink
brew install --build-from-source hyperkit
brew install --build-from-source infer

None have ARM bottles, so this is expected.

@MikeMcQuaid
Copy link
Member

Formulae where the changes weren't only style changes (but straightforward), I tried to build and test the formula locally. If we'd rather run CI here, I'll remove the CI-syntax-only label.

I'm fine with using CI-syntax-only here, personally. I trust rubocop to do these autocorrects correctly.

@nandahkrishna
Copy link
Member Author

ARM Big Sur:

Error: 4 failed steps!
brew install --build-from-source flow     # no ARM bottle
brew install --build-from-source hashlink # no ARM bottle
brew install --build-from-source hyperkit # no ARM bottle
brew install --build-from-source infer    # no ARM bottle

✅ All expected.

Intel Big Sur:

Error: 4 failed steps!
brew install --build-from-source hyperkit        # Known to be broken, no Big Sur bottle
brew test --retry --verbose hyperkit             # Same as above
brew install --build-from-source infer           # Need to check
brew install --verbose --build-bottle volatility # Python2 has no venv module, changes not related to failure (Homebrew/brew#10039)

🟡 Need to check infer.

Catalina:

Error: 4 failed steps!
brew test --verbose fn                           # Connection failure, looks like a false positive
brew install --build-from-source hyperkit        # See Big Sur comment
brew test --retry --verbose hyperkit             # Same as above
brew install --verbose --build-bottle volatility # See Big Sur comment

✅ 1 false positive, others expected.

Mojave:

Error: 4 failed steps!
brew audit molten-vk --online --git --skip-style # Network-related stable URL not reachable (re-checked, passed)
brew install --build-from-source hyperkit        # See Big Sur comment
brew test --retry --verbose hyperkit             # Same as above
brew install --verbose --build-bottle volatility # See Big Sur comment

✅ 1 network issue, others expected.

@@ -32,7 +32,7 @@ class Ocaml < Formula

pour_bottle? do
# The ocaml compilers embed prefix information in weird ways that the default
# brew detection doesn't find, and so needs to be explicitly blacklisted.
# brew detection doesn't find, and so needs to be explicitly blocked.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unsure why infer failed to build on Intel Big Sur. It was built/tested only because of ocaml, which has a syntax-only change in a comment.

If we can ignore this failure, the PR should be good to merge, I think.

Copy link
Member

Choose a reason for hiding this comment

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

Works for me. Thanks @nandahkrishna!

@nandahkrishna nandahkrishna merged commit a71271a into Homebrew:master Jul 6, 2021
@nandahkrishna nandahkrishna deleted the style-fixes branch July 6, 2021 13:41
@nandahkrishna
Copy link
Member Author

Rebased and merged. Thanks for the feedback everyone!

@github-actions github-actions bot added the outdated PR was locked due to age label Aug 13, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants