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

WIP: Add generate parameter - Fixes #211 #212

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Multiply
Copy link

@Multiply Multiply commented Nov 6, 2019

Changes

I've added a relatively crude parameter parse, that allows comma separated arguments, with multiple values delimited by +, as comma is used to distinguish between multiple parameters.

Verification

I've tested it by using a modified version of the bazel rules, following the comment here: Dig-Doug/rules_typescript_proto#10 (comment)
I couldn't seem to find any cli tests, so please guide me in the right direction, if I missed them.

@jonny-improbable
Copy link
Contributor

Thank you for raising this.

WRT code structure, can I suggest that you introduce a 'proper' type for the Parameters object instead of relying on {[key: string]: string[]} - this should make things a little more extensible/maintainable in future. I'd also suggest introducing a parseParameters() func to further improve readability.

Please could you also update the documentation? I would suggest that we replace the current references to service= with generate= and standardize on this new parameter.

@emil14
Copy link

emil14 commented May 20, 2021

How it's going? :)

@Multiply
Copy link
Author

We ended up not using this at all, after moving 100% to golang.

If anyone wants to pick it up, feel free to use my work here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants