-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
*: print-deps prints file of user's choice or by top-level file; remove --as-file #1819
*: print-deps prints file of user's choice or by top-level file; remove --as-file #1819
Conversation
bb7eb71
to
514cec3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after nit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agreed with Haseeb's comment but all in all, looks good. I won't hold up the PR for such a minor nit.
…top-level file; remove --as-file
11721cd
to
fd8a35e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of the change:
--as-file
, which is now default behaviour--dep-manager
so users can specify which type of file to print (default behavior is still old behavior)Motivation for the change: current
print-deps
without--as-file
errors because our module files contain non-versions likemaster
. These lines would need to be removed from the modfile's bytes and added back after parsing, a messy (and high LOC) process all for an output that is likely used less than that from--as-file
.--dep-manager
is a nice-to-have in case someone wants to switch dependency managers or loses their manager file somehow.Closes #1810