-
Notifications
You must be signed in to change notification settings - Fork 653
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
Config finder make
target
#1328
Conversation
Opening this up to see if people have any better ideas on how to do this (without some of the flaws) |
Neat. Can you add this to build setup? I think this along with ctags can be the same step. |
This dumps the configs at will, not into some file... Did you want this to be more visible (hence printing at the start of build-setup)? |
Hmmm good point. Printing into a file probably isn't great for discoverability. This should be added to CI somewhere though. |
6f70eab
to
efb4b7d
Compare
81e56cf
to
c39970f
Compare
e978209
to
821273c
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
2cf8c18
to
17d1b2c
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.
Somehow my review comment didn't get posted.
Can you fix the dump into /tmp? That's a bit concerning to me.
CONFIG_FRAG_LEVELS ?= 3 | ||
.PHONY: find-config-fragments | ||
find-config-fragments: $(SCALA_SOURCES) | ||
rm -rf /tmp/scala_files.f |
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'm afraid of this causing problems for shared installations. Can you make this dump the file within the repo?
Adds a new target to help users find some config fragments (also adds to the docs on how to use it).
Unfortunately, I wanted to do this through Scala reflection, however, that isn't possible since you can only look at the subclasses of
sealed
traits/classes. This is a simple solution that covers large majority of configs (assuming config. fragments are defined on a single line - "class NAME extends CONFIG\n"). This doesn't cover configs that don't inherit directly fromConfig
... I don't have time to do that right now (or figure out a better way).TODO:
class NAME extends Config\n
). Extend this to match on multi-line (is there a scala-parser in Python that we can just read the scala file and see if there is an object that matches this).Related PRs / Issues:
Type of change:
Impact:
Contributor Checklist:
main
as the base branch?changelog:<topic>
label?changelog:
label?.conda-lock.yml
file if you updated the conda requirements file?Please Backport
?