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

Do not require a DSL version in cask header #15901

Merged
merged 1 commit into from
Dec 17, 2015
Merged

Do not require a DSL version in cask header #15901

merged 1 commit into from
Dec 17, 2015

Conversation

jawshooah
Copy link
Contributor

Following up on #15782. Casks can now be created with no DSL version in the header. For example:

cask 'no-dsl-version' do
  version :latest
  ...
end

Casks with DSL versions in the header are still valid, but the DSL version will be ignored.

@jawshooah jawshooah added enhancement awaiting maintainer feedback Issue needs response from a maintainer. core Issue with Homebrew itself rather than with a specific cask. labels Dec 17, 2015
Following up on #15782. Casks can now be created with no DSL version in
the header. For example:

  cask 'no-dsl-version' do
    version :latest
    ...
  end

Casks with a DSL version in the header are still valid, but the DSL
version will be ignored.
@jawshooah
Copy link
Contributor Author

@caskroom/maintainers Any comments? If not, I'll merge at 10pm UTC.

@vitorgalvao
Copy link
Member

Why not make it a symbol as well (cask :no-dsl-version do)?

@jawshooah
Copy link
Contributor Author

@vitorgalvao We could, though I'm not sure what the benefit would be.

@jawshooah
Copy link
Contributor Author

Also, symbols can't contain hyphens unless surrounded in quotes. So it would need to be either :no_dsl_version or :'no-dsl-version'.

@vitorgalvao
Copy link
Member

Ah, no, ignore it. I was thinking of having it as symbol for consistency with what we had, but just realised I misinterpreted it. On first look it seemed like we needed to be explicit about not having a version, but I see now it stands in to be the name of the cask.

@vitorgalvao vitorgalvao mentioned this pull request Dec 17, 2015
@adidalal
Copy link
Contributor

LGTM.

After this merge, existing Casks could be changed to the new header as time permits. That would also be a (silent) indicator that a maintainer has looked at the Cask and ensured that everything checks out.

That would also be a good time to implement any other breaking changes, so it might be best to avoid a mass update (proactively) until the core stabilizes a bit more/the backlog of current PRs that deal with core functionality are merged or otherwise addressed.

@jawshooah
Copy link
Contributor Author

That would also be a (silent) indicator that a maintainer has looked at the Cask and ensured that everything checks out.

To be honest, I'm not too jazzed about combing through ~3000 casks to find bugs. A mass update would be as easy as s/cask\s+:v[\d_]+\s+=>/cask/g.

Any PRs that include truly breaking changes should also include fixes for affected casks (in separate commits).

@adidalal
Copy link
Contributor

Yeah, on second thought, maybe not the best idea. I forgot how many Casks we had!

I would say then that this PR doesn't seem to have any regressions and so a merge, followed by a mass scripted update whenever convenient should work out well.

@jawshooah jawshooah removed the awaiting maintainer feedback Issue needs response from a maintainer. label Dec 17, 2015
jawshooah added a commit that referenced this pull request Dec 17, 2015
Do not require a DSL version in cask header
@jawshooah jawshooah merged commit ceb7af6 into Homebrew:master Dec 17, 2015
@jawshooah jawshooah deleted the no-dsl-version branch December 17, 2015 23:48
tsparber added a commit to tsparber/homebrew-cask that referenced this pull request Dec 22, 2015
@miccal miccal removed core Issue with Homebrew itself rather than with a specific cask. enhancement labels Dec 23, 2016
@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