Skip to content

Conversation

tgeng
Copy link
Contributor

@tgeng tgeng commented Apr 11, 2023

The previous implementation ignores any change of source files in external repos. That implementation works for maven_install because maven_install does not download any jars into that particular repo. But it does not work for cases when the external repo comes with sources/jars in itself.

We have such use cases internally in Pinterest though I am not aware of any such use cases in common open source world. I have tested this implementation with our internal repo and it seems to work as expected. Interestingly, the difference of introduced in this PR is also sort of captured by the e2e test.

The previous implementation ignores any change of source files in
external repos. That implementation works for maven_install because
maven_install does not download any jars into that particular repo. But
it does not work for cases when the external repo comes with
sources/jars in itself.

We have such use cases internally in Pinterest though I am not aware of
any such use cases in common open source world.
@tgeng tgeng marked this pull request as ready for review April 11, 2023 03:37
@tgeng
Copy link
Contributor Author

tgeng commented Apr 12, 2023

Hi @tinder-maxwellelliott , can you take a look when you have time? Thanks!

@tgeng
Copy link
Contributor Author

tgeng commented Apr 18, 2023

Gentle ping 😄

@tinder-maxwellelliott tinder-maxwellelliott merged commit 5b29fb6 into Tinder:master Apr 18, 2023
@tinder-maxwellelliott
Copy link
Collaborator

Gentle ping 😄

Thanks again

@tgeng tgeng deleted the fix branch April 18, 2023 04:00
@tgeng
Copy link
Contributor Author

tgeng commented Apr 18, 2023

Could you create another release? Thank you!!

@tinder-maxwellelliott
Copy link
Collaborator

Could you create another release? Thank you!!

Done

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.

2 participants