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(cypress): cypress executable toolchain #2668

Merged
merged 1 commit into from
Jun 7, 2021

Conversation

mrmeku
Copy link
Collaborator

@mrmeku mrmeku commented May 11, 2021

This is a major refactor the cypress_repository rule
Prior to this change, the cypress CLI from NPM was invoked
to download the corresponding Cypress binary.

This was bad for a number of reasons. Namely it was non cachable
and did not match bazel's semantics for grabbing the right binary
for a particular platform / architecture

The new cypress_repositories rule, downloads binaries for all
platforms from Cypress's CDN and uses bazel's select logic
to use the appropriate one given a particular configuration.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

The old repository rule cypress_repository has been removed and in its place is a new cypress_repositories rule. User's will swap out these rules and add a version attribute to their new cypress_repositories workspace rule with a version which matches their package.json version of cypress.

@alexeagle
Copy link
Collaborator

Was just discussing today with @mattem how esbuild has a similar fetch problem. Seems like they could use a similar solution maybe with tool chains?

@alexeagle alexeagle added this to the 4.0 milestone May 12, 2021
@alexeagle alexeagle changed the base branch from stable to 4.x May 12, 2021 14:26
@mrmeku mrmeku force-pushed the cypress-again branch 4 times, most recently from f0ef430 to 103bab5 Compare May 18, 2021 18:39
@google-cla
Copy link

google-cla bot commented May 18, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels May 18, 2021
@purkhusid
Copy link
Contributor

Would love to see this merged, anything blocking this at the moment?

@mrmeku mrmeku requested a review from alan-agius4 as a code owner May 25, 2021 19:32
@google-cla
Copy link

google-cla bot commented May 25, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: yes and removed cla: no labels May 25, 2021
@mrmeku mrmeku force-pushed the cypress-again branch 4 times, most recently from 75883fe to e191b1d Compare May 26, 2021 15:29
@mrmeku mrmeku changed the title feat(cypress): cachable repository rule feat(cypress): cypress executable toolchain May 26, 2021
@mrmeku mrmeku force-pushed the cypress-again branch 3 times, most recently from 95a96d7 to a37511c Compare May 26, 2021 16:11
@mrmeku
Copy link
Collaborator Author

mrmeku commented May 26, 2021

@googlebot I consent.

index.bzl Outdated Show resolved Hide resolved
@mrmeku mrmeku force-pushed the cypress-again branch 2 times, most recently from 6c33e73 to 8ed8351 Compare June 7, 2021 15:03
This is a major refactor the cypress_repository rule
Prior to this change, the cypress CLI from NPM was invoked
to download the corresponding Cypress binary.

This was bad for a number of reasons. Namely it was non cachable
and did not match bazel's semantics for grabbing the right binary
for a particular platform / architecture

The new cypress_repositories rule, downloads binaries for all
platforms from Cypress's CDN and uses bazel's select logic
to use the appropriate one given a particular configuration.
@mrmeku mrmeku merged commit f1f5ee6 into bazel-contrib:4.x Jun 7, 2021
twheys pushed a commit to twheys/rules_nodejs that referenced this pull request Jan 13, 2022
This is a major refactor the cypress_repository rule
Prior to this change, the cypress CLI from NPM was invoked
to download the corresponding Cypress binary.

This was bad for a number of reasons. Namely it was non cachable
and did not match bazel's semantics for grabbing the right binary
for a particular platform / architecture

The new cypress_repositories rule, downloads binaries for all
platforms from Cypress's CDN and uses bazel's select logic
to use the appropriate one given a particular configuration.
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.

4 participants