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

gitx-rowanj: Version and checksum #6126

Merged
merged 1 commit into from
Sep 10, 2014
Merged

gitx-rowanj: Version and checksum #6126

merged 1 commit into from
Sep 10, 2014

Conversation

treyharris
Copy link
Contributor

According to #1021, versioned/checksummed downloads should be preferred when available and the URL contains the version number. This is the case with gitx-rowanj—in fact, the appcast referenced links to the versioned downloads, not the unversioned one.

According to #1021, versioned/checksummed downloads should be preferred
when available and the URL contains the version number. This is the case
with gitx-rowanj—in fact, the appcast referenced links to the versioned
downloads, not the unversioned one.
@alebcay
Copy link
Member

alebcay commented Sep 10, 2014

Great to see the results of some deep digging! Thanks! 👍

alebcay added a commit that referenced this pull request Sep 10, 2014
gitx-rowanj: Version and checksum
@alebcay alebcay merged commit 6250d1c into Homebrew:master Sep 10, 2014
@vitorgalvao
Copy link
Member

That is not what the issue says at all. It’s still open exactly because it’s undecided as of yet. Using latest (the way we started with) is definitely preferred for now (that’s the important bit). We don’t support updating yet, so having a versioned cask is actually a disadvantage.

As stated in that issue (emphasis mine), “[w]e should only transition toward versions when we have appcasts working, and other things to make the versioned Casks have an actual, tangible advantage to users over the 'latest' Casks”. That condition is not yet met, so always up-to-date urls are currently preferred.

@alebcay
Copy link
Member

alebcay commented Sep 10, 2014

So, should we revert this PR?

@treyharris
Copy link
Contributor Author

Read together with CONTRIBUTING.md, the issue seems to say the opposite.
CONTRIBUTING. md seems to suggest preferring versions where possible, and
the issue gives some pro/con bullet points as to why. If the preference is
instead, "use versions only if no unversioned download is available", it
should just say that.

I did a lot of reading in order to come to the wrong conclusion,
apparently. Something like the sentence you wrote above could just be added
to CONTRIBUTING.md along with some language in the version and sha256
descriptions saying "avoid this".

It's not clear to me why one would ever bother with the checksum if it's
preferably absent! (It seems to be legal even when the version is not
:latest, so should someone do a mass edit to remove the checksums?)

Versioning and checksums looks like the sort of hard-but-desirable grunt
work--like normalizing internal data structures or proofreading
documentation--that most projects welcome folks pitching in with. That this
is an arcane, special purpose feature is not at all clear; just look at the
examples.

Sorry, I thought I was helping out here.

@vitorgalvao
Copy link
Member

Reading your reply, I understand my comment might have sounded a bit rough. If that was the case, my apologies, as it was not the intention. That said, in the interest of clarity, I’ll just point out the answers to your concerns briefly and directly.

and the issue gives some pro/con bullet points as to why

It also gives pro/con bullet points as to why the reverse might be preferable. As I said, it is an open issue, and it is an open discussion. The idea of it is exactly to find the best option. Using latest as the preferred option was the standard at the time, so until a consensus is reached, it makes sense to not change that.

It's not clear to me why one would ever bother with the checksum if it's preferably absent!

Like appcasts, they’ll be useful later, and were implemented with the idea of thinking ahead. Versions will be more useful when we have upgrade functionality.

It seems to be legal even when the version is not :latest

It is not. :no_checksum and latest must be used together, or the audit fails.

Currently, here’s where we (somewhat unofficially) stand: latest is at the moment superior to versioned, so it is preferred. Soon, when upgrade functionality and appcasts are up-and-running, these will bring advantages that will make versioned downloads better, and then it makes sense to switch.

So, should we revert this PR?

@alebcay We should, yes. We have many precedents on similar cases.

@treyharris Thank you for the contribution. I hope my position was a bit clearer this time, and that I haven’t put you off from contributing. Again, apologies if my comment infuriated you, as it was truly not my intention. Please realise the project isn’t done, it should very much be considered an alpha — it’s incomplete and there are many things undocumented, some on purpose. Documenting something that will likely change is not a great idea, as it can lead to a lot of confusion. That is part of the reason collaborators are here: to help with this issues as they arise.

@alebcay
Copy link
Member

alebcay commented Sep 11, 2014

Alrighty, reverting this PR in #6134. First time using GitHub's built-in function, not sure if I should be excited or disappointed.

@treyharris
Copy link
Contributor Author

@vitorgalvao I'm sorry if I sounded "infuriated"—my home Internet had gone out and I was trying to reply on my phone, which led to terser unedited comments than I'd usually send.

I do think—and it sounds like you'd agree—that the current status quo as you've described it is a) probably hard to understand for the newbie contributor and b) is contradicted in the documentation.

I understand the desire not to muck with documentation about things which are in flux, but leaving now-false information in the documentation simply because it represents a previous stable state seems like it's just asking for contributor frustration.

I've prepared the following pull request to address that. I tried to stay away from any forward-looking statements and just change what I read as the encouragement to use versioned downloads.

@vitorgalvao
Copy link
Member

The confusion is not a recurring issue (although it has happened), so it was not a pressing matter. Your edits to the documentation are welcome, though, since they improve clarity. Thank you.

@treyharris treyharris deleted the gitx-rowanj branch September 13, 2014 17:53
@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