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

feat(yarn): yarn support for workspaces #4668

Conversation

easadev
Copy link
Contributor

@easadev easadev commented May 31, 2019

PR Checklist

What is the current behavior?

What is the new behavior?

Fixes/Implements/Closes #[Issue Number].

@cla-bot
Copy link

cla-bot bot commented May 31, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Esteban Sánchez.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@easadev
Copy link
Contributor Author

easadev commented May 31, 2019

PR for this #4667

@cla-bot
Copy link

cla-bot bot commented May 31, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Esteban Sánchez.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@easadev
Copy link
Contributor Author

easadev commented May 31, 2019

Seems like I need a git course. I tried amending the commit, but it resulted in a brand new commit, and I still cant get the old commit verifed.. should I delete this PR? or is there a way to fix it. Thank you and sorry for the noise

@rosen-vladimirov
Copy link
Contributor

test cli-smoke

@rosen-vladimirov
Copy link
Contributor

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Jun 5, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Esteban Sánchez.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Jun 5, 2019

The cla-bot has been summoned, and re-checked this pull request!

@rosen-vladimirov
Copy link
Contributor

test cli-plugin cli-templates cli-preview

@@ -228,7 +228,10 @@ export class PluginsService implements IPluginsService {
}

private getPackageJsonFilePathForModule(moduleName: string, projectDir: string): string {
return path.join(this.getNodeModulesPath(projectDir), moduleName, "package.json");
const pathToJsonFile = require.resolve(`${moduleName}/package.json`, {
paths: [projectDir]
Copy link
Contributor

Choose a reason for hiding this comment

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

note: this code will work with Node.js 8.9 or later.

@rosen-vladimirov
Copy link
Contributor

@easadev thanks a lot for your contribution. We'll run some tests and merge your PR. Meanwhile, it looks like one of your commits is with incorrect username, can you please squash all commits and push-force the branch, so we can ensure the CLA is signed.

@easadev easadev force-pushed the feat/cli-4667/support-yarn-workspace branch from cfaf494 to 019a051 Compare June 6, 2019 01:15
@cla-bot cla-bot bot added the cla: yes label Jun 6, 2019
@easadev
Copy link
Contributor Author

easadev commented Jun 6, 2019

Hi, @rosen-vladimirov Thank you for reviewing this so quickly. I just squashed the commits and force pushed them as you suggested.

@rosen-vladimirov
Copy link
Contributor

test cli-plugin cli-templates cli-preview

@easadev
Copy link
Contributor Author

easadev commented Jun 15, 2019

Hi @rosen-vladimirov I noticed that one check failed (ci/jenkins/nativescript-cli) but I cant see the details. Can I know what it was so that I can fix it? Thank you

@rosen-vladimirov
Copy link
Contributor

test cli-plugin

@rosen-vladimirov rosen-vladimirov merged commit 8722364 into NativeScript:master Jun 25, 2019
@rosen-vladimirov
Copy link
Contributor

Hey @easadev ,
Thank you very much for your contribution and please excuse me for the delayed reply. I've rerun the tests and they pass, so I've merged the PR. The feature will be included in the next official release (6.0.0).
Thanks a lot!

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

Successfully merging this pull request may close these issues.

2 participants