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

Move: excludes arg paths should be relative to source, not destination #48

Merged
merged 3 commits into from
Aug 30, 2018

Conversation

jkwlui
Copy link
Member

@jkwlui jkwlui commented Aug 29, 2018

transforms.move(
    source,
    destination,
    excludes=['garbage.md']) # after this change, this will be relative to the source, not destination

So if there exists a source/garbage.md, it will be ignored.

Please let me know if this change makes sense to you.

@jkwlui jkwlui requested a review from crwilcox August 29, 2018 19:41
@jkwlui jkwlui self-assigned this Aug 29, 2018
@jkwlui jkwlui added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Aug 29, 2018
@jkwlui
Copy link
Member Author

jkwlui commented Aug 29, 2018

This is needed for https://github.com/googleapis/nodejs-firestore/blob/master/synth.py

firestore moved their source location to under dev/

@jkwlui jkwlui merged commit 3119da9 into master Aug 30, 2018
@jkwlui jkwlui deleted the move-to-dest-excludes-relative-to-source branch August 30, 2018 22:49
igorbernstein2 added a commit to igorbernstein2/synthtool that referenced this pull request Apr 25, 2019
googleapis#48 introduced a bug where the exclude path (which is relative to a tracked path) is compared to a source path (which is relative to the source root).
This issue was unnoticed because it works ok when the source path has a single component like in firestore:
https://github.com/googleapis/nodejs-firestore/blob/89afae/synth.py#L19

But breaks when the source path has multiple components like in bigtable:
https://github.com/googleapis/google-cloud-java/blob/264643/google-cloud-clients/google-cloud-bigtable/synth.py#L30

And when the cwd doesn't match the tracked path
@igorbernstein2 igorbernstein2 mentioned this pull request Apr 25, 2019
busunkim96 pushed a commit that referenced this pull request Apr 29, 2019
* Fix move excludes

#48 introduced a bug where the exclude path (which is relative to a tracked path) is compared to a source path (which is relative to the source root).
This issue was unnoticed because it works ok when the source path has a single component like in firestore:
https://github.com/googleapis/nodejs-firestore/blob/89afae/synth.py#L19

But breaks when the source path has multiple components like in bigtable:
https://github.com/googleapis/google-cloud-java/blob/264643/google-cloud-clients/google-cloud-bigtable/synth.py#L30

And when the cwd doesn't match the tracked path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants