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

Add rubiTrack 3.app v3.4.4 #5346

Merged
merged 1 commit into from
Jul 15, 2014
Merged

Add rubiTrack 3.app v3.4.4 #5346

merged 1 commit into from
Jul 15, 2014

Conversation

jconley
Copy link
Contributor

@jconley jconley commented Jul 15, 2014

No description provided.

@vitorgalvao
Copy link
Member

Thank you for the submission, and for making url reusable. We could take this one step further by changing link to link "rubiTrack #{version.gsub(/\..*/, '')}.app".

@radeksimko
Copy link
Contributor

@vitorgalvao I'm not sure if such change is complicating the yet so simple DSL.

@jconley
Copy link
Contributor Author

jconley commented Jul 15, 2014

@vitorgalvao I can reluctantly add it this time, but instead of using .gsub in many formulas, would you be open to:

  1. making version a proper object or extended string so you can call #{version.major}
  2. making link glob-friendly so you can call: link '*.app' (and maybe consider if this could be the default if no link directive is given)

@vitorgalvao
Copy link
Member

Both your positions are extremely valid, and I’ll try to answer them with some historic reasoning when applicable. Please keep in mind, that as I stated before, historic reasoning “is a context, not an argument. The project has already changed so much that old rules definitely benefit being revisited by new eyes”.

I'm not sure if such change is complicating the yet so simple DSL.

I certainly understand your concern. I don’t feel strongly either way in this case, although making the change would make sense when the software gets updated to a new major version, and we move this to caskroom versions. I was a strong advocate “in the early days” of keeping casks as simple as possible, especially for new contributors, but truth is the ability to run code inside them is a major feature, and should be taken advantage of. We’re not making new users understand all the peculiarities of the possibilities of submissions (that’s why collaborators are here, to iron out the details). In my experience, contributors are actually very receptive to new tricks they had no idea were possible (or accepted/encouraged).

making version a proper object or extended string so you can call #{version.major}

Sounds like a great idea, actually, but perhaps only in theory. In practice, we’d have to see how useful it really is, since no gsub substitution is the norm. The one I suggested in this issue is used extremely rarely (is it used at all, elsewhere?). Much more common is to substitute . with _, for example, and there are others. At what point does implementing a rule for every little detail become too much? As per the comment linked above, the fact we can leverage runnable ruby code in the cask itself is an advantage. You’re suggesting we extensively complicate the code base to marginally improve readability. It is certainly an interesting proposition, and I like the idea very much. However, it does have flaws and requires much consideration, as most cases are exceptional, which would make implementing this unreasonable.

and maybe consider if this could be the default if no link directive is given

It doesn’t work. That was exactly how howebrew-cask worked at first, but too many edge-cases made matters complicated. Eventually, it became clear it has to be explicitly set.

@radeksimko As a final note, your comment reminded me of one of @passcod’s comments in one of the linked issues, that I’ve always kept in mind while reviewing submissions. One I’ve since then kept in mind, and that has proven to be exceptionally accurate over the growth of the project — “It'll always be complex for new users, @vitorgalvao, remember when you started :)”.

@jconley
Copy link
Contributor Author

jconley commented Jul 15, 2014

@vitorgalvao added the change, but used #{version.to_i}, does the same according to rdoc and is a bit cleaner vs .gsub

@vitorgalvao
Copy link
Member

So good. So simple, yet so effective. Definitely one of those “wish I though of that” changes. Thank you. If you have any questions/comments regarding my previous post, please feel free to say so. Fresh eyes are very much welcome is such a rapidly growing project.

Merging this.

vitorgalvao added a commit that referenced this pull request Jul 15, 2014
@vitorgalvao vitorgalvao merged commit 09060a6 into Homebrew:master Jul 15, 2014
@rolandwalker
Copy link
Contributor

@jconley I already started on making things more glob-friendly in #5043, but I wasn't happy with the interface in that PR – I think it should be more like

link expand('rubiTrack*.app')

Even better would be

link 'rubiTrack*.app'.expand

but at least at the moment we can't make that work. @vitorgalvao is right that glob behavior can't be the default.

As to version methods, that's a good idea, thanks. Having the Cask language be clean makes it more welcoming to newcomers, which is extremely important, since newcomers write most of our Cask content. Even if we can't cover every case, somewhat cleaner is somewhat better.

The idea of re-using version numbers aggressively is new. Best to let that project run for a little while, then look for common patterns we can abstract.

@vitorgalvao
Copy link
Member

@jconley Oh, right, also meant to mention @rolandwalker was working on making it glob-friendly, but forgot. My apologies for the oversight. Was hard enough to find context for those old issues (writing this on the bus, going back home), so it slipped my references.

@jconley jconley deleted the rubitrack branch July 16, 2014 14:10
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants