The Wayback Machine - https://web.archive.org/web/20220427140700/https://github.com/tuist/tuist/issues/4326
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

graph command doesn't filter targets for json format #4326

Open
GermanVelibekovHouzz opened this issue Mar 31, 2022 · 4 comments
Open

graph command doesn't filter targets for json format #4326

GermanVelibekovHouzz opened this issue Mar 31, 2022 · 4 comments
Labels
good first issue type:bug

Comments

@GermanVelibekovHouzz
Copy link

@GermanVelibekovHouzz GermanVelibekovHouzz commented Mar 31, 2022

Describe the bug
When tuist graph <target> is run for a json format no target filtering is done but all project/workspace structure is shown inside generated JSON file. Works correctly with png and dot format.

To Reproduce
Run tuist graph <som-target> -f json. Result: All projects and targets returned.

  • Tuist Version: 3.0.1
@GermanVelibekovHouzz GermanVelibekovHouzz changed the title graph command doesn't filter targets in json format graph command doesn't filter targets for json format Mar 31, 2022
@danyf90 danyf90 added type:bug good first issue labels Apr 1, 2022
@danyf90
Copy link
Contributor

@danyf90 danyf90 commented Apr 1, 2022

I can confirm the issue.
e.g. To reproduce in the tuist project tuist graph TuistDependencies -f json also includes the tuist target.

Feel free to open a PR if you want, and let me know if you need any help 🚀

@GermanVelibekovHouzz
Copy link
Author

@GermanVelibekovHouzz GermanVelibekovHouzz commented Apr 1, 2022

I can confirm the issue. e.g. To reproduce in the tuist project tuist graph TuistDependencies -f json also includes the tuist target.

Feel free to open a PR if you want, and let me know if you need any help 🚀

Thank you @danyf90. As this might be my first PR to the tuist project, I'd be grateful to get some guidance about the area to look for the issue and run/test/contribution process. 🙏

@danyf90
Copy link
Contributor

@danyf90 danyf90 commented Apr 1, 2022

Sure 🚀

The core of the graph logic is here.
In particular, the case .json: is not considering the targetsToFilter parameter.
I think it would be enough to move the logic to filter the target before that switch, and then pass the "pruned" graph in both the switch cases.
Let me know (even on Slack) if you have any doubt 🚀

Side note: also, I would also remove the . at the end of Graph exported to …, as it makes harder to copy and paste the path.

@danyf90
Copy link
Contributor

@danyf90 danyf90 commented Apr 1, 2022

And make sure to have a look at the contributors guide 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue type:bug
Projects
None yet
Development

No branches or pull requests

2 participants