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

implement argument passing #56

Closed
wants to merge 1 commit into from
Closed

Conversation

jayong93
Copy link

If pier supports passing arguments to the scripts, it would be flexible. So I have tried to implement.

I wanted to split alias and arguments which are combined into a Vec<String> in Cli structure, but when I tried, the binary always crashed. I would happy if someone implements that :).

implement argument passing

fix a collision between alias and subcommands

remove unused structures
@BenSchZA
Copy link
Member

Awesome! Thanks for the contribution @jayong93 :) I know @Plommonsorbet created this issue thread #37 a while ago where we were discussing this. I'm sure he'll be able to give a good review. It's about time we implemented this feature, so I'm glad you got the ball rolling. I'll take a look at the PR this weekend too and see what the issue is.

@Plommonsorbet
Copy link
Member

Hey, thanks for getting started on this feature, like @BenSchZA mentions I think there is a discussion to be had for how we actually want this to look like. That conversation is best had in the related issue though.

However when we have the implementation details I'd be happy to walk you through implementing the feature and adding the appropriate tests so that this feature can be totally your contribution. :)

Some points that came to mind in general that we should pro

  • Contributor's guide, how to build, debug run tests etc.
  • Add pipeline for running tests on pull requests.

@Plommonsorbet
Copy link
Member

Implemented in #62 .

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

Successfully merging this pull request may close these issues.

None yet

3 participants