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

Build: Update minimum engine versions for node and yarn #1329 #1340

Closed
wants to merge 6 commits into from
Closed

Build: Update minimum engine versions for node and yarn #1329 #1340

wants to merge 6 commits into from

Conversation

Jeykairis
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

The version of engines are >=8.9.0 (<9.0.0) for node and >=1.3.2 <2.0.0 for yarn.

Issue Number: #1329

What is the new behavior?

The version of engines are >=10.9.0 <11.0.0 for node and >=1.9.2 <2.0.0 for yarn.

Does this PR introduce a breaking change?

[] Yes
[x] No

Other information

@coveralls
Copy link

coveralls commented Sep 23, 2018

Coverage Status

Coverage decreased (-0.01%) to 88.385% when pulling 82e618d on Jeykairis:issue-1329 into 41a0d45 on ngrx:master.

@Jeykairis
Copy link
Author

It looks like bazel install the old version of yarn (v1.3.2) https://circleci.com/gh/ngrx/platform/3531?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link .
How can we specify at bazel to install to the new one (v1.9.2)?

There is also versions specified here https://github.com/ngrx/platform/blob/master/projects/ngrx.io/tools/examples/shared/package.json .
Should they be updated?

@timdeschryver
Copy link
Member

timdeschryver commented Sep 23, 2018

Try changing the node_repositories in WORKSPACE (in the root) with the following:

node_repositories(
    package_json = ["//:package.json"],
    preserve_symlinks = True,
    node_version = "10.9.0",
    yarn_version = "1.9.2",
)

package.json Show resolved Hide resolved
@timdeschryver
Copy link
Member

Hmmm seems like it's still broken, sadly I'm not a bazel expert 😅
Could you try the following, if that doesn't work I think we'll have to upgrade bazel to 0.17 (just like in the Angular repo).

node_repositories(
  package_json = ["//:package.json"],
  preserve_symlinks = True,
  node_version = "10.9.0",
  yarn_version = "1.9.2",
  node_repositories = {
    "10.9.0-darwin_amd64": ("node-v10.9.0-darwin-x64.tar.gz", "node-v10.9.0-darwin-x64", "3c4fe75dacfcc495a432a7ba2dec9045cff359af2a5d7d0429c84a424ef686fc"),
    "10.9.0-linux_amd64": ("node-v10.9.0-linux-x64.tar.xz", "node-v10.9.0-linux-x64", "c5acb8b7055ee0b6ac653dc4e458c5db45348cecc564b388f4ed1def84a329ff"),
    "10.9.0-windows_amd64": ("node-v10.9.0-win-x64.zip", "node-v10.9.0-win-x64", "6a75cdbb69d62ed242d6cbf0238a470bcbf628567ee339d4d098a5efcda2401e"),
  },
  yarn_repositories = {
    "1.9.2": ("yarn-v1.9.2.tar.gz", "yarn-v1.9.2", "3ad69cc7f68159a562c676e21998eb21b44138cae7e8fe0749a7d620cf940204"),
  },
)

@Jeykairis
Copy link
Author

Hmmm seems like it's still broken, sadly I'm not a bazel expert 😅
Could you try the following, if that doesn't work I think we'll have to upgrade bazel to 0.17 (just like in the Angular repo).

It's the first time I see bazel in action. I checked the Angular repo and it run explicitly bazel v0.17 in the Dockerfile.
Here it looks that "@bazel/typescript" "^0.15.0" is the version used related to "@angular/bazel@^6.1.0".
By upgrading to the latest "@angular/bazel": "^6.1.8" we only get "@bazel/typescript": "0.15.3".

So how can we upgrade it?

@timdeschryver
Copy link
Member

Another cause could be that it's cached. I propose we wait on @brandonroberts 's input.

@brandonroberts
Copy link
Member

You have to bump the ngcontainer in the .circleci/config.yml file to get a newer version in CI. This may require updating yarn, node and Bazel all at once.

@Jeykairis Jeykairis closed this Oct 7, 2018
@Jeykairis Jeykairis reopened this Oct 7, 2018
@Jeykairis
Copy link
Author

Thank you @brandonroberts I made some more try but the discover of Bazel is a bit fuzzy.
Could you point out what's wrong and where can I found useful information to learn more of Bazel?

@timdeschryver
Copy link
Member

Hi @Jeykairis , could you allow maintainers to push to this branch?
I wanna try something 😅

@Jeykairis
Copy link
Author

Hi @timdeschryver, sure it should be the case.

@timdeschryver
Copy link
Member

I believe we're going to need some assistance here 😅

@brandonroberts
Copy link
Member

I haven't forgot about this one. I'll take a look soon.

@brandonroberts
Copy link
Member

Wait until this lands in angular/angular before proceeding. It will provide some guidance on best practice for updating. angular/angular#26488

@brandonroberts
Copy link
Member

@Jeykairis this turned out to be a lot more changes. I included your changes in #1354. Thanks!

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.

4 participants