-
Notifications
You must be signed in to change notification settings - Fork 57
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
Add vsce pre-release step to CI #399
Conversation
Awesome! How does versioning work here? I'm not fully read up on pre releases in the marketplace. |
Well, I did not account for that yet, but I guess we also need to have version numbers for pre-releases, separated from main releases. What rust-analyzer does is basically what is also suggested by the docs, i.e. having a version where the minor part is an odd number that is always one higher than the minor part of the main release. E.g. the pre-release version for rescript-vscode now would be
|
4e81d6d
to
afdc342
Compare
Here is a chart with the full workflow for a merge into master (not for a PR build): flowchart TD
A[pull request gets merged into master branch]
A-->B[build analysis binaries]
subgraph ci [ ]
subgraph textNode1 [ci.yml]
style textNode1 fill:none,stroke:none
end
B-->C[fetch current highest version - 1.4.0]
C-->D[bump-version.js script increments version - 1.4.1]
D-->E[package VSIX with vsce - 1.4.1]
E-->F[prepublish VSIX with vsce - 1.4.1]
E-->G[add packaged VSIX to releases page - Latest master]
end
F & G --> Done
|
afdc342
to
703ac4e
Compare
@zth @cristianoc feel free to review and roast me! |
@fhammerschmidt how does one test this? With a PR on top of this PR that enables for that branch? |
Yes, you can create a branch and add it's name to the push branches in ci.yml. However, as the important bits happen on the master branch, it may be easier in a fork, or all the There are now two tokens at play here:
|
Can we test this in the current namespace we have? I'm thinking it'd be good to do that, get one final release out, and then move to our new real Marketplace ReScript org. Would that essentially be merging this and ensuring the marketplace token is available? |
I think so, but I cannot guarantee it. But I guess using the old namespace as a testbed makes a lot of sense. We can then continue on a clean slate when we figured the whole process out. |
703ac4e
to
549abee
Compare
We're a little behind with new releases. Don't know if this has to complete before a new release is done. |
It is completely independent of main releases, which will still be done manually after this PR gets merged. |
@fhammerschmidt is this understanding correct:
...? |
549abee
to
f7272d9
Compare
Ad 1.: Yes, but first add the token and then merge it, the next master build should automatically publish a pre-release then. Ad 2.: In a subsequent push to master, it will retrieve the most recent version again which would then be the pre-published Ad 3.: The only thing with regards to main releases is that you need to keep in mind that we would be forced to use the odd minor numbers for them, while using the even ones for pre-releases, e.g.
The bump-version script actually missed the case when there is a higher (major or minor) version in package.json, but not in the marketplace, I fixed that now. But I also realized, that it would be better to start from an even number main release, like I hope that was not too confusing. |
40830cc
to
d5105ca
Compare
@fhammerschmidt no that's great! I'm going to get the token sorted and we can merge this. |
@zth I really think it is inconsistent when we start from 1.3.0. Maybe we can first release a 1.4.0 and merge it afterwards? If you however don't care that the pre-releases have even minor numbers for 1.x.x but odd numbers for 2.x.x (and every subsequent major bump) then I rest my case. |
@fhammerschmidt sorry, missed the last part of your message. Yes, I agree. Let's get a |
@fhammerschmidt setting up the token + merging this tomorrow. |
Turns out I don't have permissions to produce that marketplace token. Pinging @ryyppy who might have more luck in that regard. |
@fhammerschmidt just created a |
Cool. @zth If you want I can create another PR just to test if the token works in CI (using https://fig.io/manual/vsce/verify-pat). |
Go for it! |
Ok, unfortunately you need to do a I tried this with a dummy token: ➜ rescript-vscode git:(vsce-test) npx vsce verify-pat chenglou92 --pat lol
ERROR The Personal Access Token verification has failed. Additional information:
Error: {"$id":"1","innerException":null,"message":"TF400813: Resource not available for anonymous access. Client authentication required.","typeName":"Microsoft.TeamFoundation.Framework.Server.UnauthorizedRequestException, Microsoft.TeamFoundation.Framework.Server","typeKey":"UnauthorizedRequestException","errorCode":0,"eventId":3000}
EDIT: It may still work with the correct token. Trying it in #499. |
@fhammerschmidt what's the status now, are we waiting for @ryyppy to reply in the other PR? |
As you like. I believe he was or is still sick and was not able to answer. |
So, to clarify: if you want to be sure the token is valid, you should wait for Patrick. Otherwise feel free to pull the trigger. |
@fhammerschmidt mind rebasing on master again? Inclined to merge this after that and we'll just try to fix whatever comes up if things don't work like we want to. |
I am currently on mobile. Could do it in a couple of hours. |
d5105ca
to
53262f3
Compare
Rebased! |
Fire in the hole! |
This is a first step to having automatic pre-releases published in the VSCode marketplace.Somebody with access rights needs to create a
secrets
environment in this GitHub repository and add (one of) the ReScript team's Personal Access Token (PAT) as an environment variable, namedMARKETPLACE_TOKEN
.EDIT:
This PR is basically done, but there is probably still room for some fine-tuning. As mentioned above, someone with write access needs to provide the token(s).
Also the
chenglou92.rescript-vscode
needs to be updated to whatever will be the VSCode publisher name afterwards.