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

Fix self-referential dependencies when publishing test fixtures #7485

Conversation

StefanBratanov
Copy link
Contributor

PR Description

As suggested by @jframe

bratkartoffel/security-jwt@48a7e84

Fixed Issue(s)

N/A

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

Copy link
Contributor

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

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

LGTM

@StefanBratanov StefanBratanov merged commit f752e26 into Consensys:master Sep 6, 2023
@StefanBratanov StefanBratanov deleted the fix_self_referential_dependencies branch September 6, 2023 09:36
@Nashatyrev
Copy link
Contributor

Relates to #7444

There are two points about this fix:

  1. It returns back publishing test-fixture artifacts which is good
  2. It returns back needless extra dependencies (derived from test-fixture scope) for the main artifact

I don't actually see any crime regarding the (2) except a bunch of unused dependencies are to be downloaded and appended to the classpath.

@StefanBratanov
Copy link
Contributor Author

Relates to #7444

There are two points about this fix:

  1. It returns back publishing test-fixture artifacts which is good
  2. It returns back needless extra dependencies (derived from test-fixture scope) for the main artifact

I don't actually see any crime regarding the (2) except a bunch of unused dependencies are to be downloaded and appended to the classpath.

Didn't realise about (2) annoying really, we will see if there is another workaround, but given we actually have downstream consumers using the test-fixtures it is preferrable to be published.

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.

3 participants