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

Missing docstring on proxy flag #141

Closed
MitchellBerend opened this issue Oct 15, 2022 · 4 comments · Fixed by #142
Closed

Missing docstring on proxy flag #141

MitchellBerend opened this issue Oct 15, 2022 · 4 comments · Fixed by #142
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers hacktoberfest https://hacktoberfest.com/
Milestone

Comments

@MitchellBerend
Copy link
Collaborator

#[derive(Debug, PartialEq, Eq)]
pub struct Config {
/// Directory to store all locally downloaded tools
pub store_directory: String,
pub proxy: Option<String>,
/// Info about each individual tool
pub tools: BTreeMap<String, ConfigAsset>,
}

#[derive(Parser, Debug)]
#[command(author="Dmitrii Kovanikov <[email protected]>", version, about="A CLI tool to manage other CLI tools", long_about = None)]
pub struct Cli {
/// Sets a path to a configuration file (default: $HOME/.tool.toml)
#[arg(short, long, value_name = "FILE")]
pub config: Option<PathBuf>,
#[arg(short, long, value_name = "uri")]
pub proxy: Option<String>,
#[command(subcommand)]
pub command: Command,
}

The person who implemented this flag (me) forgot to add a doc strings to explain how to use this and what it does.
This option takes a proxy that can be understood by the ureq Proxy struct.

@MitchellBerend MitchellBerend added documentation Improvements or additions to documentation good first issue Good for newcomers labels Oct 15, 2022
@chshersh chshersh added the hacktoberfest https://hacktoberfest.com/ label Oct 15, 2022
@chshersh chshersh added this to the v0.3.0: Auto milestone Oct 15, 2022
@jim4067
Copy link
Contributor

jim4067 commented Oct 15, 2022

let me look into this

@MitchellBerend
Copy link
Collaborator Author

Sure go ahead. I'm not sure what the doc string on this should be to be honest. I only know how to make the proxy work with my own set up.

The original issue is #82 and the pr is #121

@chshersh
Copy link
Owner

I would simply add a usage example, e.g.:

You can custom proxy in tool-sync like this:

tool --proxy=http://127.0.0.1:8080 sync

@jim4067
Copy link
Contributor

jim4067 commented Oct 15, 2022

Thanks for the suggestion. I think I'll go with it.

Unrelated, but I was also thinking of updating the config flag from continuous present tense to present tense.

From -c, --config <FILE> Sets a path to a configuration file (default: $HOME/.tool.toml)
to -c, --config <FILE> Set a path to a configuration file (default: $HOME/.tool.toml)

chshersh added a commit that referenced this issue Oct 21, 2022
Resolves #141

### Additional tasks

- [ ] Documentation for changes provided/changed
- [ ] Tests added
- [ ] Updated CHANGELOG.md

Co-authored-by: Dmitrii Kovanikov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers hacktoberfest https://hacktoberfest.com/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants