Skip to content

cask audit: check for sourceforge appcast#4278

Merged
commitay merged 3 commits intoHomebrew:masterfrom
commitay:audit-sourceforge-appcast
Jun 7, 2018
Merged

cask audit: check for sourceforge appcast#4278
commitay merged 3 commits intoHomebrew:masterfrom
commitay:audit-sourceforge-appcast

Conversation

@commitay
Copy link
Copy Markdown
Contributor

@commitay commitay commented Jun 5, 2018

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

@commitay commitay added the cask Homebrew Cask label Jun 5, 2018
@commitay commitay requested a review from a team June 5, 2018 06:48
@ghost ghost assigned commitay Jun 5, 2018
@ghost ghost added the in progress Maintainers are working on this label Jun 5, 2018
Copy link
Copy Markdown
Contributor

@claui claui left a comment

Choose a reason for hiding this comment

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

Minor nitpicks for clarity and readability.

Comment thread Library/Homebrew/cask/lib/hbc/audit.rb Outdated
return if cask.version.latest?
return unless cask.url.to_s =~ %r{sourceforge.net/(\S+)}

add_warning "Cask hosted on SourceForge, please add an appcast. See https://github.com/Homebrew/homebrew-cask/blob/master/doc/cask_language_reference/stanzas/appcast.md"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor language tweak: Download is hosted on SourceForge to avoid confusion.

(Adding the is would be just for consistency with the GitHub warning, which is also a complete sentence.)

describe "SourceForge appcast check" do
let(:error_msg) { /Cask hosted on SourceForge/ }

context "when the Cask is not hosted on SourceForge" do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For clarity, replace Cask by download here, too.

end

describe "SourceForge appcast check" do
let(:error_msg) { /Cask hosted on SourceForge/ }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Change this to :appcast_warning or :appcast_message to make your assertions more readable, and because it’s not an actual error.

@commitay commitay merged commit bb7ec94 into Homebrew:master Jun 7, 2018
@ghost ghost removed the in progress Maintainers are working on this label Jun 7, 2018
@commitay commitay deleted the audit-sourceforge-appcast branch June 7, 2018 00:42
@commitay commitay removed their assignment Jun 7, 2018
@lock lock bot added the outdated PR was locked due to age label Jul 7, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cask Homebrew Cask outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants