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

ext_code and ext_str are inconsistent with ext_code_files and ext_str_files #101

Open
mariusgrigoriu opened this issue Feb 27, 2019 · 4 comments

Comments

@mariusgrigoriu
Copy link
Contributor

ext_code and ext_str take a map of variable names and values, but ext_code_files and ext_str_files use parallel arrays. In my opinion, the map is a much better experience since it is possible for the user to accidentally transpose values in a larger list without noticing.

Any reason to keep the parallel arrays or would it be ok to switch everything to a map?

I was getting started on a PR to #98 but this style issue needs to be resolved prior to introducing
more inconsistency.

@mariusgrigoriu
Copy link
Contributor Author

I would be happy to contribute a PR if the maintainers are ok with using a map for all of the args such as ext_code_files and ext_str_files

@Globegitter
Copy link
Collaborator

Globegitter commented May 22, 2019

@mariusgrigoriu The main reason I could not implement what you proposed was due to a limitation in bazel/starlark: bazelbuild/bazel#1232. But if you do find a better way of implementing this I would be very happy to get a PR in.

@Globegitter
Copy link
Collaborator

I actually did think of some improvement of this a while ago, because it also becomes a bit weird when one uses a filegroup. I was thinking one way of improving this is that if one only provides the ext_str_files or ext_code_files without the corresponding ext_str_file_vars/ext_code_file_vars it could choose the name automatically based on the file name. Which in would probably be fine for >90% of the cases.

@Globegitter
Copy link
Collaborator

But either way, happy to get in any improvements you come up with.

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

No branches or pull requests

2 participants