Skip to content
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

update schedule.json #2757

Closed
wants to merge 5 commits into from
Closed

update schedule.json #2757

wants to merge 5 commits into from

Conversation

SEWeiTung
Copy link
Contributor

@SEWeiTung SEWeiTung commented Nov 10, 2019

  1. According to our plan in https://nodejs.org/en/about/releases/, we've now published the 13th version of Nodejs, add this to the schedule for the maintance.

  2. Add the missing coding name for v12.

@SEWeiTung SEWeiTung requested a review from a team November 10, 2019 06:29
@SEWeiTung SEWeiTung changed the title add v13 to the schedule.json update schedule.json Nov 10, 2019
1. According to our plan in https://nodejs.org/en/about/releases/, we've
now published the 13th version of Nodejs, add this to the schedule for
the maintance.

2. Add the missing coding name for v12.
@XhmikosR XhmikosR requested a review from Trott November 10, 2019 14:49
@Trott
Copy link
Member

Trott commented Nov 10, 2019

Looks good to me but I never even noticed this file before (that I can remember anyway) so I'll leave the actual approving to others (which has already happened anyway).

@XhmikosR

This comment has been minimized.

@Trott

This comment has been minimized.

@XhmikosR

This comment has been minimized.

@nschonni
Copy link
Member

Should the build just curl this file from https://github.com/nodejs/Release/blob/master/schedule.json instead of keeping a separate copy?

@SEWeiTung
Copy link
Contributor Author

SEWeiTung commented Nov 11, 2019

@nschonni: Not sure yet, but according to your description, it seems this file here is duplicated?
@nodejs/release: It seems this file has been defined in the https://github.com/nodejs/Release/blob/master/schedule.json, so do we have to maintain this copy here? Just for a confirm.
If you think this file can be removed, I'll remove it.

@richardlau
Copy link
Member

I think the file in the repository is a placeholder that gets overridden during build by load-schedule (called by deploy):

nodejs.org/package.json

Lines 12 to 14 in 575a47e

"deploy": "npm-run-all load-schedule build:deploy external:survey gzip",
"load-versions": "node scripts/load-versions.js",
"load-schedule": "curl -sS https://raw.githubusercontent.com/nodejs/Release/master/schedule.json -o source/schedule.json",
(see #1890)

@XhmikosR
Copy link
Contributor

Can't we remove the folder and the file completely?

@SEWeiTung
Copy link
Contributor Author

SEWeiTung commented Nov 12, 2019

I think the file in the repository is a placeholder that gets overridden during build by load-schedule (called by deploy):

nodejs.org/package.json

Lines 12 to 14 in 575a47e

"deploy": "npm-run-all load-schedule build:deploy external:survey gzip",
"load-versions": "node scripts/load-versions.js",
"load-schedule": "curl -sS https://raw.githubusercontent.com/nodejs/Release/master/schedule.json -o source/schedule.json",

(see #1890)

So this file should be totally removed, or just run the script to update the file, just like what I submitted?

@XhmikosR
Copy link
Contributor

This patch is moot since this is overwritten on build time. We should instead see if we can just remove the folder and the file.

@XhmikosR XhmikosR closed this Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants