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: add a script to scaffold and bootstrap a new package #4984

Merged
merged 1 commit into from
Apr 18, 2020

Conversation

raymondfeng
Copy link
Contributor

@raymondfeng raymondfeng commented Mar 28, 2020

We often need to add new packages to loopback-next. Copy/paste is error prone. This script scaffolds a new project under packages or extensions depending on the current directory. Then it uses lerna bootstrap to set up dependencies.

Depends on #4994

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@raymondfeng raymondfeng requested a review from bajtos as a code owner March 28, 2020 20:18
@raymondfeng raymondfeng added the Internal Tooling Issues related to our tooling and monorepo infrastructore label Mar 28, 2020
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

I love the idea of automating the task of creating new packages, I agree the current "copy & paste" process is too error prone.

I feel it's important to write some tests to verify what content the new package contains and even more importantly to verify that it's correctly registered in our monorepo (see https://github.com/strongloop/loopback-next/blob/master/docs/site/DEVELOPING.md#adding-a-new-package).

Without that, I don't see much value in this script, when compared to running lb4 extension && lerna bootstrap manually.

bin/add-extension-package.js Outdated Show resolved Hide resolved
bin/add-extension-package.js Outdated Show resolved Hide resolved
bin/add-extension-package.js Outdated Show resolved Hide resolved
bin/add-extension-package.js Outdated Show resolved Hide resolved

console.log('Adding project %s/%s...', location, name);

// Run `lb4 extension`
Copy link
Member

Choose a reason for hiding this comment

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

What if we want to add a regular package (packages/{name}) or even an acceptance test (acceptance/{name}) - is lb4 extension a good command to use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For regular package, yes. But not for acceptance tests.

@bajtos bajtos added the developer-experience Issues affecting ease of use and overall experience of LB users label Mar 30, 2020
@raymondfeng raymondfeng force-pushed the add-extension-script branch 2 times, most recently from 4467e86 to 69096b9 Compare March 30, 2020 17:55
@raymondfeng
Copy link
Contributor Author

Without that, I don't see much value in this script, when compared to running lb4 extension && lerna bootstrap manually.

Please see updated DEVELOPING.md. The script automates most steps except updating CODEOWNER and MONOREPO.md. But we do print out a message after the command finishes.

@raymondfeng raymondfeng requested a review from bajtos March 30, 2020 17:59
@raymondfeng
Copy link
Contributor Author

I'll add the copyright step once #4994 is landed.

@raymondfeng
Copy link
Contributor Author

raymondfeng commented Mar 30, 2020

I feel it's important to write some tests to verify what content the new package contains and even more importantly to verify that it's correctly registered in our monorepo (see https://github.com/strongloop/loopback-next/blob/master/docs/site/DEVELOPING.md#adding-a-new-package).

Not sure if we want to write tests for internal scripts.

@raymondfeng raymondfeng force-pushed the add-extension-script branch 10 times, most recently from 17e1a56 to 9fa601c Compare April 2, 2020 03:21
@dhmlau dhmlau added this to the April 2020 milestone Apr 2, 2020
@bajtos
Copy link
Member

bajtos commented Apr 3, 2020

Thank you for the updates, I'll take another look next week.

Copy link
Contributor

@emonddr emonddr left a comment

Choose a reason for hiding this comment

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

Perhaps update the issue title to match the commit title? To me, they seem like different things. thx.
@raymondfeng ^

@raymondfeng raymondfeng force-pushed the add-extension-script branch from 9fa601c to bfb5ead Compare April 6, 2020 21:03
@raymondfeng raymondfeng mentioned this pull request Apr 6, 2020
7 tasks
@raymondfeng raymondfeng force-pushed the add-extension-script branch from bfb5ead to 25f3cc5 Compare April 7, 2020 21:10
@raymondfeng
Copy link
Contributor Author

@bajtos Since it's an internal script. I went ahead to merge it. Please continue to provide feedback so that we can improve it if necessary.

@raymondfeng raymondfeng merged commit 61669bd into master Apr 18, 2020
@raymondfeng raymondfeng deleted the add-extension-script branch April 18, 2020 17:31
3. Tidy up the project

- Remove unused files
- Rename `tsconfig.json` to `tsconfig.build.json`
Copy link
Member

Choose a reason for hiding this comment

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

@raymondfeng I think this is no longer accurate, now that we are using project references (via #5155)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Internal Tooling Issues related to our tooling and monorepo infrastructore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants