This repository has been archived by the owner on Jul 4, 2023. It is now read-only.
Add description field to Formulae #39911
Closed
Closed
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ef39f39
Add a desc field to Formula
adzenith e4499dd
ack: add description
adzenith 11b089b
Info shows the desc if present
adzenith 85da0a5
`brew search` can take a `--desc` argument to search descriptions
adzenith 33f9222
audit looks for and validates 'desc'
adzenith bb6a599
`brew create` adds a `desc` field to the new formula
adzenith c71654f
example-formula.rb has a 'desc'
adzenith File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,6 +244,32 @@ def audit_options | |
end | ||
end | ||
|
||
def audit_desc | ||
# For now, only check the description when using `--strict` | ||
return unless @strict | ||
|
||
desc = formula.desc | ||
|
||
unless desc and desc.length > 0 | ||
problem "Formula should have a desc (Description)." | ||
return | ||
end | ||
|
||
# Make sure the formula name plus description is no longer than 80 characters | ||
linelength = formula.name.length + ": ".length + desc.length | ||
if linelength > 80 | ||
problem "Description is too long. \"name: desc\" should be less than 80 characters (currently #{linelength})." | ||
end | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe also check whether desc is empty. As sometimes people forget to change the desc filed created by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea. Fixed! |
||
if desc =~ %r[[Cc]ommandline] | ||
problem "It should be \"command-line\", not \"commandline\"." | ||
end | ||
|
||
if desc =~ %r[[Cc]ommand line] | ||
problem "It should be \"command-line\", not \"command line\"." | ||
end | ||
end | ||
|
||
def audit_homepage | ||
homepage = formula.homepage | ||
|
||
|
@@ -706,6 +732,7 @@ def audit | |
audit_file | ||
audit_class | ||
audit_specs | ||
audit_desc | ||
audit_homepage | ||
audit_deps | ||
audit_conflicts | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,13 @@ def search | |
exec_browser "https://admin.fedoraproject.org/pkgdb/packages/%2A#{ARGV.next}%2A/" | ||
elsif ARGV.include? '--ubuntu' | ||
exec_browser "http://packages.ubuntu.com/search?keywords=#{ARGV.next}&searchon=names&suite=all§ion=all" | ||
elsif ARGV.include? '--desc' | ||
query = ARGV.next | ||
Formula.each do |formula| | ||
if formula.desc =~ query_regexp(query) | ||
puts "#{formula.name}: #{formula.desc}" | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use |
||
elsif ARGV.empty? | ||
puts_columns Formula.names | ||
elsif ARGV.first =~ HOMEBREW_TAP_FORMULA_REGEX | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
A minor question here. Which style is better? Personally, I prefer
homepage
is at the top.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.
The homepage is more canonical, so to speak, but the desc is more relevant (to me as someone who doesn't know what the formula is). That's why I put the desc first. Obviously it's easy to move second if that's preferred. I put it first in
brew create
to match this, but can easily move it all below.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.
My own personal strong preference is that we put the
desc
as the last line of the initial block, i.e. under either thesha
orrevision
. For some reason, having it as the top line is making me very uncomfortable, but I'm just weird.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 would prefer to keep it above the sha and version, because the sha and version apply to a single version of the software, whereas the description deals with the software regardless of version (like the homepage). Is there a reason you'd want to have it down there?
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.
For me at least, the initial formula block is in order of things I need to know most, that are most important to most people/formula authors.
I want to know where the formula is from first, I want to know where it's being download from next, and then I want to know what the checksum is for security. The formula description feels less important to me than any of those three things.
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 first was in favor of “homepage then desc then the rest” but I read @adzenith’s comment and now agree with the “description first, then homepage, then the rest”. The
desc
is indeed the more relevant here, and the homepage’s URL doesn’t usually give you much information about the software (say the formula is “foobar”, the URL will likely befoobar.sourceforge.net
orsome-academic-website.edu/~doe/software/foobar
) and you can open it quickly in your browser withbrew home <formula>
.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.
My two cents: Let's please not let this PR get bogged down with bike-shedding. As far as I know,
brew
does not enforce any rules about the order of items in the top of the class. So I can't see why we need to worry so very much about it. (My point being that once we start asking for opinions, everyone will have one, but really: is it that important?)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.
This isn't quite true. It's not in the
audit
but the only things that move around in the initial block unless there's need forstable do
is theversion
line, if there is one. That can go either below the URL or below thesha
. But generally, although not audit-enforced, deviation from that tends to get asked to change. Whether that's just because formulae become a pain to update/compare/etc if all of them have different layouts or whether that's for another reason, I defer to the people who wrote the implementation.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 didn't realize. Thanks for clarifying.
In that case, I suppose if we could get a quick ruling from the team, that would be great. I don't think it much matters where they pick, but I agree that having a convention is useful.
Again, my goal is to avoid protracted delay of the PR being merged because of bike-shedding.
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.
Yes, let's hold off the ordering discussion for now and just leave it as-is.