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 Adobe Air to 14.0 #4914

Merged
merged 1 commit into from
Jun 17, 2014
Merged

Update Adobe Air to 14.0 #4914

merged 1 commit into from
Jun 17, 2014

Conversation

radeksimko
Copy link
Contributor

I was also thinking about using the version stanza as a variable in the URL, but I'd have to move the version above url and before I do that, I'd need to know if there's any prefered order of stanzas in general. I have found nothing in the documentation or anywhere else.

@vitorgalvao
Copy link
Member

Historically, we have preferred a certain consistency in the order of stanzas, yes. Personally, I’ve advocated for it multiple times. That said, such ideals made much more sense when we had just a few stanzas, shared across all casks; nowadays we have a ton of powerful options to use, and keeping some type of consistency is only possible to a certain degree. Some casks do not respect this undocumented order, sometimes with good reason (the case you mention, for example).

I’ve submitted a few pull requests today (they’re still open, if you care to check) exactly with this question in mind, #4910 being the big one. It seems to me that currently (as opposed to when the project started) it’s important to separate stanzas in a way that makes more sense when updating casks, as that is the most common action. This approach has the advantage of making future changes easier, with less room for mistakes.

#4910 changes 32 casks (the ones that use link and when unpacked have inside a directory with a version number, which is sometimes overlooked). If this change makes sense as is welcomed by the other maintainers, I’d say we could start changing others.

@rolandwalker
Copy link
Contributor

These are good points. Moving toward a preferred order is reasonable, but only if we remain considerate toward first-time Cask authors. For example, I don't think we should refuse a Cask because it doesn't follow the order.

@vitorgalvao
Copy link
Member

Agreed. Nor should we refuse them if they don’t make the type of changes we’re talking about (changing version dynamically, in the url). We should accept them, and possibly offer to help with such a change, after this is documented. More often than not, new contributors (particularly the ones that have very little programming experience) are glad when learning these kinds of small details, in homebrew-cask.

@vitorgalvao
Copy link
Member

@radeksimko On another note, it seems like you’re adding AIR’s dmg on this commit.

rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Jun 16, 2014
@radeksimko
Copy link
Contributor Author

Patch updated - version & sha moved to the top, version reused, ready to go.

@vitorgalvao
Copy link
Member

AdobeAIR.dmg is still included. Shouldn’t that be removed?

@rolandwalker
Copy link
Contributor

Test suite should be amended to fail if an unknown file is seen in the toplevel directory.

@radeksimko
Copy link
Contributor Author

AdobeAIR.dmg is still included. Shouldn’t that be removed?

Ah, now I see... I just totally missed that one... the diff is very brief in case of binary files.
Sorry!

Fixed.

rolandwalker added a commit that referenced this pull request Jun 17, 2014
@rolandwalker rolandwalker merged commit 16775cd into Homebrew:master Jun 17, 2014
@radeksimko radeksimko deleted the adobe-air branch June 17, 2014 10:21
@vitorgalvao
Copy link
Member

the diff is very brief in case of binary files.

It is. I only noticed because the “files changed” tab showed “2”, instead of “1”.

rolandwalker added a commit to rolandwalker/homebrew-cask that referenced this pull request Jun 17, 2014
@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.

3 participants