Understanding the GitHub comparison links on the upgrade page

Something that has bugged me for a little while, but which I have not mentioned until now, is that the links provided on the upgrade screen don’t allow for easy comparison of the currently-installed commit vs the new-version commit.

In the image above, the first link goes here:
https://github.com/discourse/discourse/compare/efcea14...tests-passed

And the second link goes here:
https://github.com/discourse/discourse/compare/69aa8f1...tests-passed

My suggestion is also generating a link to the comparison of those two commits, like: https://github.com/discourse/discourse/compare/efcea14...69aa8f1

1 Like

I just realized the first link and my suggested link are essentially identical. Would this always been the case, even if I get behind by more than one version?

Yes, the first link is your current commit compared to the head of tests-passed, which is what your upgrade will be. Never fully understood what the second link is for myself…

3 Likes

You already have exactly what you’re asking for.

The other version comparsion is comparing the tests passed label to the latest checkin at discourse/discourse, thus the only way that would be different is if someone just checked in something that causes the tests to fail.

Furthermore, the only way it would remain different is if the build was not fixed for an extended period of time, meaning more and more stuff was checked into latest and none of it fixed the build by getting the tests to pass. This is rare, but it does happen from time to time.

It is a valid comparison, in my opinion.

So there’s nothing to do here, I don’t think either of these requests are actionable? Shall I close this topic @abrambailey?

1 Like

Sorry just re-read what @jomaxro said, and he answered my question. Thanks, yes you can close this.

2 Likes

Sorry, this is broken now, I noticed it today

We are comparing something nonsensical here, the intention was to show the difference between my current commit and the commit I will be upgrading to

In practice we show nonsense for the second link, hence it should not be a link

Or the reverse … something has to give… but linking to:

https://github.com/discourse/discourse/compare/69aa8f1...tests-passed when I am 490 commits behind is bananas, especially when the upgrade button is below the link

Wait… what do you mean? It’s 100% valid on my colocated Discourse install.

The version of Discourse per the meta tag in view source:

<meta name="generator" content="Discourse 2.2.0.beta6 - https://github.com/discourse/discourse version 7b253dbe4a92f7656bbc779d68e07ec93c56dfed">

the dashboard upgrade reports the currently installed local version of Discourse as

discourse (7b253db)

which is completely consistent with the version number in the meta tag above ↑ and the remote version is reported as

Remote Version: 69aa8f1

which is consistent with the latest checkin at https://github.com/discourse/discourse/commits/master ↓ ↓

It is acting “correctly”, it is just mega super confusing. I hit it today when upgrading reviews and just assumed there was some bug.

Or maybe … most simple thing is NOT to link commit hashes anymore and instead only hyperlink the words “490 new commits”

6 Likes

Oh I see what you’re saying. The links should be swapped, so the upgrade button is closer to the “exactly how many changes will I get and what will they be” link.

That’s kind of a different argument, but OK, sure.

5 Likes

This is now complete and wonderful. Significantly less confusing.

11 Likes

I would suggest the repository on the left linking to the public github repo though :wink:

4 Likes

Sure … @Osama when you do the ember upgrade can you take that? Also, the styling on the commit hashes is too strong should be lighter grey, plus… maybe we should try for friendlier names if available: v2.2.0.beta6 +100 and so on… not possible for all repos, but where it is … it would be very handy.

5 Likes

Really like the visual change @sam, so much clearer.

However, it looks like we switched from 3-dot diff to a 2-dot diff, was that intentional? I personally much prefer being able to review the list of commit messages (https://github.com/discourse/discourse/compare/7e1f20b...1d62d3d) rather than simply seeing the changes made (https://github.com/discourse/discourse/compare/7e1f20b..1d62d3d).

6 Likes

Sure, @osama can change that when he gets around to updating … I did not make that particular change on-purpose

5 Likes

This was completed ~3 weeks ago, I forgot to update here :sweat_smile:

8 Likes

This topic was automatically closed after 2 days. New replies are no longer allowed.