Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(developers.md): update node version as specified in package.json #16384

Merged
merged 2 commits into from
Dec 29, 2017

Conversation

skryvets
Copy link
Contributor

@skryvets skryvets commented Dec 26, 2017

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Docs update

What is the current behavior? (You can also link to an open issue here)

DEVELOPERS.MD contains different node version from package.json. Yarn install works with 8.9.1+ only and doesn't work with 6.x as specified in docs.

What is the new behavior (if this is a feature change)?

Docs are updated in accordance with package.json.

Does this PR introduce a breaking change?

Not.

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Fix/Feature: Docs have been added/updated
  • Fix/Feature: Tests have been added; existing tests pass

Other information:
Sorry, this might be a very small and not significant fix, but I guess still requires some attention.

@Narretz
Copy link
Contributor

Narretz commented Dec 29, 2017

Hi, thanks for the PR. This is not 100% correct, though - the version we support is ^8.9.1, which means any 8.x.x version is supported. Could you adjust the PR accordingly?

@Narretz Narretz added this to the Backlog milestone Dec 29, 2017
@skryvets
Copy link
Contributor Author

Hi, @Narretz. Sorry, I misunderstood.

Could you adjust the PR accordingly?

Sure. I just pushed my changes.
Thank you a lot for explaining why is that :-)

@Narretz Narretz merged commit e942e1e into angular:master Dec 29, 2017
@Narretz
Copy link
Contributor

Narretz commented Dec 29, 2017

@sergeome Thanks, merged. Do you mean why 8.x is correct, but 8.9.1 is not? It's because the ^ means a range of acceptable releases, starting with 8.9.1, but also for example 8.10 if that were ever released ... (see https://stackoverflow.com/a/22345808/787333). Which makes a realize, 8.x isn't exactly correct, because the minimum is 8.9.1. But that's okay for now.

@skryvets
Copy link
Contributor Author

That was my initial point,

I cloned the repo, looked at the docs and it didn't work for me. I noticed that in package.json there is node ^8.9.1:

...
"engines": {
    "node": "^8.9.1", <- This guy
    "yarn": ">=1.3.2",
    "grunt": "^1.2.0"
  },
...

Whereas in the docs it was 6.x.

I agree that 8.x is still more correct than 6.x :-)

Anyways, will adjust next time, when would create another PR if this item will have a place.
Thank you, @Narretz!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants