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.

Sign up for GitHub

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

Jump to bottom

test: cleanup user_managed_deps e2e test #1428

Merged
merged 1 commit into from
Dec 6, 2019
Merged

test: cleanup user_managed_deps e2e test #1428

merged 1 commit into from
Dec 6, 2019

Conversation

gregmagolan
Copy link
Collaborator

@gregmagolan gregmagolan commented Dec 6, 2019 •
edited
Loading

  • e2e/user_managed_deps now tested in its own circleci job from within the working directory; I tried using working_directory in bazelci but for some reason the node_modules filegroup glob was always empty
  • remove the old dormant test_legacy_runfiles circleci job
  • remove @e2e_packages workspace as it is not being used anymore

@gregmagolan gregmagolan force-pushed the cleanup_e2e_tests branch 3 times, most recently from e6854a6 to 1bb8a6b Compare December 6, 2019 07:31
@gregmagolan
test: cleanup user_managed_deps e2e test
9d7a656 
* e2e/user_managed_deps now tested in its own circleci job from within the working directory; I tried using working_directory in bazelci but for some reason the node_modules filegroup glob was always empty
* remove the old dormant test_legacy_runfiles circleci job
* remove @e2e_packages workspace as it is not being used anymore
@gregmagolan
Copy link
Collaborator Author

Pre-req for #1426 as user_managed_deps is causing problems with that PR

alexeagle
alexeagle approved these changes Dec 6, 2019
View reviewed changes
Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

That's not good that we had the legacy_external_runfiles test but it wasn't getting run.

Why does the user_managed_deps example have a new typescript dependency? It doesn't seem to have any typescript code.

@alexeagle alexeagle merged commit 9ea9f75 into bazel-contrib:master Dec 6, 2019
@gregmagolan
Copy link
Collaborator Author

Why does the user_managed_deps example have a new typescript dependency? It doesn't seem to have any typescript code.

I shall address that question in the leaky transitive deps section of the Why Not To Have Bazel Dependencies lightning talk at BazelCon next week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexeagle alexeagle alexeagle approved these changes

@soldair soldair Awaiting requested review from soldair

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gregmagolan @alexeagle @googlebot