Add support for latest release #323
Conversation
|
Couldnt get the tests running locally despite setting the GITHUB_TOKEN, not sure whats wrong there |
|
Id change the meaning of “latest” to “stable”, ie not introduce a new concept. GitHub releases can be marked as latest, eg: https://api.github.com/repos/erlang/otp/releases/latest and I think it is useful to maintain parity with that. this would remove ability to get latest published RC, as it works right now, but honestly I’m not sure if it is that useful. If people want to test against the bleeding edge they probably should test against main branch anyway. curious what others think! |
I fully agree with this. Maybe latest-stable? |
|
So just rename what is currently Because neither versions exactly match the GH release that is marked as latest, as per the readme entry for the current |
My 2 🪙, I find this behaviour useful, though not a common case. I have a library that uses private Elixir compiler internals, and I test against edge Elixir to get ahead of breaking changes. Right now I use master, but my preference would be to just run on RCs, to ignore false positives that a breakage is coming, so it is useful to me. Only started looking into changing over to RCs today, but will hold off until a decision is made here. 🙂 |
Yeah, it's a bit tricky, there's ambiguity in both terms, which led to this to begin with. I suppose it comes down to expectations, which is also why I suggested perhaps latest (current behavior) and latest-stable or just stable. Just stable on its own is a bit more ambiguous, but we shouldn't aim for perfection either. Would like to hear what people think of that. |
|
To me, "latest" means latest release, which stands in contrast to a pre-release/release candidate—which is definitively not a release, so should not be in the "latest release" category. So using both "stable" and "latest" would be confusing to me in this context. My vote would be for a "stable" vs "edge" dichotomy, each a different classification of latest release. We could soft-deprecated "latest" and keep its current behaviour, until a cut-off date/version. |
|
I think "stable" and "edge" make sense to me. The most important part should be that they are properly documented. |
625f243 to
77fe764
Compare
|
I have now made it so that |
|
I think I'm good on stable and edge. I'd like to hear what @wojtekmach thinks. |
|
Fixed the main error from the CI |
|
Not sure about the failure of ubuntu / OTP 27, Elixir , Gleam 1.5.1, rebar3 that seems to be unrelated to my changed. The unit test failure changes i assume come from this test case, but i dont quite know where exactly. And unfortunately i couldnt get the tests running locally, so i cant easily debug this |
|
Ok, i fixed the first issue with the Unfortunately i am not sure if i can just remove that requirement? |
|
I'd still like to have the ability to test "edge" (e.g. not deprecating it as the PR suggests). Whatever naming comes out of it (parity with other resources, e.g. GitHub, as @wojtekmach mentions, is Ok, but we can also build on top of it). So, whether it's
|
|
The current idea for the PR is to add a Also an alias for the current |
|
I don't have a strong opinion but I'm not 100% sold on "edge" meaning "latest published released including RCs". I mean, it can mean anything and be documented as such, but my immediate association would be with nightly builds, the bleeding edge, way more unstable than an RC. @paulo-ferraz-oliveira mentioned in the ticket:
A big +1 for |
|
I am completely impartial to the naming and how you want the readme entry to look. I only care that there is the functionality o get the latest non-RC version |
|
I agree with @wojtekmach. |
|
So change the current (as in for this PR) |
|
I'm good with those, for the time being. I'll let others pitch in, too, though. |
|
+1 from me! |
|
Done, hopefully i didnt mess anything up in the rename, but tests did pass locally. |
|
As I wrote this feature initially, then I would consider not returning rc in latest a breaking change. However, I still don’t understand what the issue is with breaking actions. As this was specifically developed with the intention of something breaking rather that be somewhere in cldr or any other place. I do see the benefit in getting the most recent major version. Today it is already possible to retrieve the latest minor and patch releases by only setting the major version. So there might be an opportunity to leverage that to set a minimum major version e.g +27 |
|
I think that, even if it can be considered breaking, this comment by @wojtekmach supports our decision to have
For Erlang/OTP Edit: later, @Schultzer clarified, and well, that it's not just Erlang/OTP that takes that input, so the parity argument loses some relevance. Where we discussed this initially I asked "And would we consider release candidates?", and your reply was "RC is welcome but unnecessary"; it seems to us that having it clearer as |
Correct, but I think we’re forgetting that the context of that comment was about getting the feature landed, and I was willing to not include RC if for some reason that would be technically impossible or resistant towards its. So it was still intentional that it included RC in the end. |
|
I'm personally Ok to release:
|
|
Personelly I would prefer a major bump, since there is no bug in latest and we won't break anyone who is expecting rc to be included, people that would like the new behavior can easily upgrade to 2. where we would have both latest and latest-rc. From my perspective then this is the best of both worlds, and what most developers would expect. After all it is right in the readme and part of the test cases:
|
|
I want to float an alternative that may satisfy all interested parties. Why not keep latest as is and add an additional option for excluding certain types of releases? This would keep latest as is and avoid a breaking change (behavioral) and give us a way to solve the original issue. After all, all proposed solutions add a new additional option, yet as stated this avoids concerns with breaking changes. Thoughts? |
I like that, and it keeps it very clean with an option! |
This is only partially correct. The options are still the same (which is why @starbelly, mind you we already have a lot of options, for an action that's supposed to do one thing only (install a CI environment for OTP + optional language):
(I'm aware most of them have defaults) What would the scope of the option be? Let's suppose such an option is called I think:
@wojtekmach had a good suggestion about including a What's new in the release notes and keep at 1.x. Thus:
|
I hear what you're saying. I think we can all agree has nice api has as few options as possible and just does the right thing 90% of the time. I'd like to figure out how to reduce our number of options, yet that would require a 2.x, and a digression from the convo. What I'm seeing though is the addition of a new option I will yield to consensus, but I did want to try and offer up a simple solution that will keep everyone happy, based on all the commentary that I was indeed following. Ergo, I'm ok with going with a breaking behavioral change if we are loud about it. |
I think the option is properly the best stop gap, until there is a clearer idea about what to do with the options in general, it might be the best option (pun intended) since it forces us to think real hard about improving the overall API and deal with them for a 2. Introducing a breaking change or bump just for latest-rc seams equally bad in hindsight. @starbelly thank you for suggesting this alternative that can satisfy all, until there is a good consensus about what is a good API for 2.x. I don’t think we should rush this, and a stop gap unblock everyone without breaking anything. It’s fairly easy to add the option for your workflow if you don’t want rc in certain workflows. Hindsight is always 20/20. I wish I saw this as clear as you! |
|
Ok, so let's try to think about the option... what would that be?, and how would it apply to both Do you mean having a value like Edit:
I don't think it is easy. As I stated above, if e.g. your option is |
Something just hit me, maybe all of this is completely unnecessary, as we’re using https://www.npmjs.com/package/semver I believe you could simply define your version as >0 or >27 and get the latest major version. Keep in mind that I haven’t tested this and I don’t see anything in our test suite. I think it is important to verify if we could solve this by adding a test and some more documentation for this functionality. |
|
I'd be Ok, as per your latest reply, if:
|
Regardless of all of this, I think having proper tests for this functionality is a requirement, and we can reference them in the readme, for anyone who might be in doubt of how the resolution works. |
Description
I added an additional version specifier of
stable, that works just likelatestexcept that it ignores RC versions.Another way to do this is to directly adjust
latestto also ignore those, but the tests are currently very explicit about RCs being favoured over previous stable versions so i opted for this approach.Closes #<issue>.