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

monorepo: overhaul lerna/yarn/repo tools #2984

Merged
merged 10 commits into from
Nov 9, 2020

Conversation

joshgummersall
Copy link
Contributor

  • Clean up lerna.json
  • Move to yarn workspaces
  • Move to yarn for CI
  • Add botbuilder-repo-utils with update versions script
  • Introduce preview and deprecated package.json metadata for more consistent and simple build & release

The intent of this set of changes is to simplify as much as possible.

The updateVersions.ts script is the brains of the release versioning step. This script now encodes and formalizes a conventional use of two separate Azure DevOps variables and a pretty gnarly sed script.

Yarn is a lot more performant than npm install - CI time is cut almost in half by this change alone. Yarn also produces a single lock file (yarn.lock) that ensures that all developers have a consistent set of installed packages. It also serves as a wonderful input to a cache key calculation.

The CI steps are extracted into shared templates and used more consistently in the various CI jobs.

Note: I'd like to drop the tools sub-tree entirely but it's currently used for a single botframework-connector test.

@joshgummersall joshgummersall force-pushed the jpg/lerna-yarn-workspaces branch from 64b81b5 to 34d8135 Compare October 30, 2020 21:48
@joshgummersall
Copy link
Contributor Author

I plan to handle moving the functional/streaming test scenarios to yarn as well. Eventually 😄

@joshgummersall joshgummersall force-pushed the jpg/lerna-yarn-workspaces branch from 34d8135 to f2def63 Compare October 30, 2020 22:35
@joshgummersall
Copy link
Contributor Author

Still need to add/update docs that reference npm instead of yarn.

Josh Gummersall added 2 commits November 2, 2020 08:28
- Clean up lerna.json
- Move to yarn workspaces
- Move to yarn for CI
- Add botbuilder-repo-utils with updateVersions script
@joshgummersall joshgummersall force-pushed the jpg/lerna-yarn-workspaces branch from f2def63 to 9dfc3a4 Compare November 2, 2020 16:28
@stevengum
Copy link
Member

@johnataylor, @munozemilio, @EricDahlvang, @Zerryth, @mdrichardson FYI

@mdrichardson
Copy link
Contributor

mdrichardson commented Nov 2, 2020

@joshgummersall Regarding:

Note: I'd like to drop the tools sub-tree entirely but it's currently used for a single botframework-connector test.

Is that the test using nock-helper? If so, I've got a related issue for that. Sounds like you probably have some ideas how to clean that up. I'd be happy to take that on if we can nail down some design guidelines for it.

@joshgummersall
Copy link
Contributor Author

@mdrichardson let's have that discussion over here: microsoft/botframework-sdk#5977

@joshgummersall joshgummersall force-pushed the jpg/lerna-yarn-workspaces branch from 33f10dd to 617606e Compare November 2, 2020 23:19
@joshgummersall
Copy link
Contributor Author

Fixes #1383

@joshgummersall joshgummersall force-pushed the jpg/lerna-yarn-workspaces branch from e76fcba to 9de05af Compare November 4, 2020 00:31
Copy link
Member

@stevengum stevengum left a comment

Choose a reason for hiding this comment

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

Approving latest changes! (9de05af)

@Zerryth
Copy link
Contributor

Zerryth commented Nov 4, 2020

I plan to handle moving the functional/streaming test scenarios to yarn as well. Eventually 😄

Sounds good to me! @joshgummersall


Also regarding the E2E streaming tests, to keep in mind with your yarn overhaul:

Background

The React app's base was created from from create-react-app command, which defaults to yarn, which is what was originally what we had in the az. pipeline, however--and this was just lack of deeper knowledge about yarn--we switched it to npm when we were working on the patch release that included Content Security Policy updates, because we knew how to coax the dependency tree into picking up the right DLJS, and in turn the correct bf-streaming version.

React App Dependencies

  • react-app
    • bf-webchat
      • pinned version of dljs
        • pinned version of bf-streaming
    • custom dljs
      • nightly version bf-streaming

I couldn't figure out how to tell yarn to not use the pinned version of webchat and to instead use the custom dljs created by the pipeline--it kept on picking up the pinned version, which didn't have the latest streaming bits
And given the time crunch and the fact that the rest of the bb-js repo used npm, we switched react app to npm, since we knew how to mess with the dependency tree there for the tests.


That being said, I have full confidence that you thankfully know yarn a lot better than I do, and can switch everything over 🎊 🙌

@joshgummersall
Copy link
Contributor Author

@stevengum and @munozemilio I just pushed one last commit that supports more complicated versioning scenarios. It's worth taking a quick look at the repo utils work and the daily build YAML to make sure I'm not doing anything crazy.

Copy link
Member

@stevengum stevengum left a comment

Choose a reason for hiding this comment

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

Still approving 👍

I had some questions regarding status code and another for import style and fs/promises

Copy link
Member

@stevengum stevengum left a comment

Choose a reason for hiding this comment

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

Approving latest commit (3ba7142) and responses to my last review. :shipit:

@joshgummersall joshgummersall merged commit 6103ccf into main Nov 9, 2020
@joshgummersall joshgummersall deleted the jpg/lerna-yarn-workspaces branch November 9, 2020 20:20
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.

5 participants