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
Build: Add CI release scripts (fxies #6884) #6885
Conversation
Thanks for the pull request, @nzakas! I took a look to make sure it's ready for merging and found some changes are needed:
Can you please update the pull request to address these? (More information can be found in our pull request guide.) |
LGTM |
LGTM, but waiting another day for others to look |
LGTM. |
These particular changes may not be necessary - let's hold off so I can verify. |
LGTM |
LGTM |
@@ -650,7 +651,7 @@ target.gensite = function(prereleaseVersion) { | |||
} | |||
const added = versions.added[baseName]; | |||
|
|||
if (!versions.removed[baseName] && !test("-f", sourcePath)) { | |||
if (!versions.removed[baseName] && !fs.existsSync(sourcePath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly different logic here, right? Before it was checking if it exists and if it's a file, not it just checks if it exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, not sure how this got in. Removing.
LGTM |
LGTM |
What issue does this pull request address?
#6884
What changes did you make? (Give an overview)
ci-release
scriptAddedgh-release
scriptIs there anything you'd like reviewers to focus on?
Ensure changes in
Makefile.js
make sense.