pulsar-edit
[core] Stop handling `beta`, `nightly`
This PR ended up being much simpler than expected.
But it's goal was remove any explicit handling of beta or nightly for Pulsar in reference to release channels.
Since Pulsar has not used that nomenclature for over a year, it seemed past due time.
In it's place, I have added dev, since for our dev instance of Pulsar that is what it's currently recognized as, which makes sense since that version is only accessible when literally in dev mode.
What may be worth mentioning is this change doesn't address the already non-existent distinction between rolling and regular release channels. But that wasn't handled previously so this is no change in that respect. Here we just remove what's become useless code.
I assume the rest of the code that referenced nightly and beta was in the auto update code that we pruned a while ago.
Ahh @Daeraxa you are probably right there, so makes sense why there was so little still referencing it
@DeeDeeG You are absolutely correct, in that the changes adding dev are made for in development Pulsar versions. Not ever released versions.
I thought it'd be akin to beta but you are right that there's a distinct difference between a released version and everything else. If anything we could change this to detect rolling releases and non-rolling releases. But for the purpose of this PR do you think this logic should be even more simplified?
I reckon that, coincidentally for each of these separately, it makes the most sense to remove beta and nightly without replacing them with anything.
(Tangent: If we do remove handling for beta and nightly that is, since who knows if some fork would want to roll those release channels, and being able to handle them here doesn't actively hurt anything much. But this would clean up a few lines of code and run perhaps some nanoseconds faster, I dunno.)
FWIW I don't see us using getCommandNameForChannel() anywhere internally in core repo, so if it's not meant to be a public API, or usable by packages, etc., we can delete the function entirely. [EDIT to add: I tried and I can't do atom.getCommandNameForChannel() in Pulsar Dev Tools, so it seems like it's not a public API. Might as well delete the function entirely if it's unused?]
isReleasedVersion() is clearly marked as being a public API, maybe some packages use it. But I don't see us currently using it anywhere ourselves either, at least not internally in core repo. So, I think we can't delete this without it technically being semver major / breaking (?).
But by and large these functions are both kind of obsolete without there being the full-on "release channels" concept supported by us anymore, where we flavor/style the app based on the channel name.
FWIW I don't see us using
getCommandNameForChannel()anywhere internally in core repo, so if it's not meant to be a public API, or usable by packages, etc., we can delete the function entirely. [EDIT to add: I tried and I can't doatom.getCommandNameForChannel()in Pulsar Dev Tools, so it seems like it's not a public API. Might as well delete the function entirely if it's unused?]
I think that function was entirely used internally via the update scripts previously. I don't think there's any use for it outside of that
isReleasedVersion()is clearly marked as being a public API, maybe some packages use it. But I don't see us currently using it anywhere ourselves either, at least not internally in core repo. So, I think we can't delete this without it technically being semver major / breaking (?).
Could probably just give it the Grim deprecate treatment and remove it in a version or two? (Since there are larger problems with the way that Atom was set up to be a 1.y.z-only application we can't ever do a true Semver major)
We could put a grim notice on it, but the fact we can never actually do a 2.0 means I think we should plan that there is effectively no deadline for package authors to act on the grim notifications... unless we intend to do small breaking things in the same semver major, which I don't think is worth it. So it's kind of strange to deprecate things and also plan to never remove them. I think we just make isReleasedVersion() a very simple function and not deprecate, as there is no sincere intent to remove... right?
I'd be willing to push changes to the branch simplifying the functions as per above feedback if that would be okay, at which point I believe this would be ready to merge IMO.
EDIT: This assuming no objections from other reviewers either of course.
@DeeDeeG If you have the time to push changes to get this one ready to go that would be totally fine by me, and I'd appreciate it!