Skip to content

Remove npm references from Contributing #1390

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

Merged
merged 1 commit into from
Jan 4, 2020
Merged

Remove npm references from Contributing #1390

merged 1 commit into from
Jan 4, 2020

Conversation

afontcu
Copy link
Member

@afontcu afontcu commented Jan 3, 2020

Closes #1235 (sorta...)

Right now, npm is not a valid option as the project won't run properly. Only yarn is. Thus, I updated the docs accordingly.

In an ideal world, running VTU scripts with npm would "just work", but I'm not experienced enough with Lerna to identify what's missing. That's why I thought that, at least, we should make the docs reflect the current state of the art.

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems will still reference npm in various places, including release scripts and package.json. We might want to move all those to yarn as well. What do you think?

@afontcu
Copy link
Member Author

afontcu commented Jan 3, 2020

Seems will still reference npm in various places, including release scripts and package.json. We might want to move all those to yarn as well. What do you think?

Hm, I didn't want to change the release scripts as they are working fine (AFAIK). Same thing with package.json scripts as long as they are run using yarn. Should we update them anyway?

@lmiller1990
Copy link
Member

Your call - I think it makes sense to pick and stick w/ a single package manager. It's basically as easy as s/npm run/yarn/g, but we can do it in a separate PR if you are more comfortable with that too.

@lmiller1990
Copy link
Member

Since moving the scripts from npm to yarn is (maybe) a more significant change, let's chat about it and do that in another PR. This is fine for now, nice job.

@lmiller1990 lmiller1990 merged commit e7b8d50 into dev Jan 4, 2020
@lmiller1990 lmiller1990 deleted the remove-npm-docs branch January 4, 2020 12:00
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.

Cannot run repo locally without yarn - BLOCKING CONTRIBUTIONS
2 participants