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

Don't require 'exe_name' #55

Closed
chshersh opened this issue Aug 30, 2022 · 0 comments · Fixed by #104
Closed

Don't require 'exe_name' #55

chshersh opened this issue Aug 30, 2022 · 0 comments · Fixed by #104
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@chshersh
Copy link
Owner

chshersh commented Aug 30, 2022

Currently, tool-sync requires to specify exe_name in the configuration. However, it often happens that the tool has exactly the same name as the repository. In this case, we could allow to not specify the exe_name field and default it to the repo name.

To achieve this, we should patch the tool configuration in relevant places (and add tests as well):

Relevant code:

  • /// Configure 'ToolInfo' completely from 'ConfigAsset'
    fn full_configure(config_asset: &ConfigAsset) -> Option<ToolInfo> {
    let owner = config_asset.owner.clone()?;
    let repo = config_asset.repo.clone()?;
    let exe_name = config_asset.exe_name.clone()?;
    Some(ToolInfo {
    owner,
    repo,
    exe_name,
    asset_name: AssetName {
    linux: config_asset.asset_name.linux.clone(),
    macos: config_asset.asset_name.macos.clone(),
    windows: config_asset.asset_name.windows.clone(),
    },
    })
    }
  • exe_name: config_asset
    .exe_name
    .clone()
    .unwrap_or_else(|| self.exe_name.clone()),
@chshersh chshersh added enhancement New feature or request good first issue Good for newcomers labels Aug 30, 2022
@chshersh chshersh added this to the v0.2.0: Improved sync milestone Aug 30, 2022
@chshersh chshersh pinned this issue Sep 2, 2022
mmohammadi9812 added a commit to mmohammadi9812/tool-sync that referenced this issue Sep 9, 2022
@chshersh chshersh unpinned this issue Sep 20, 2022
chshersh added a commit that referenced this issue Sep 20, 2022
Resolves #56

I've improved the documentation for `tool-sync` in a few ways:

* Rewrote README
* Added a CHAGELOG
* Slightly changed the CONTRIBUTING.md guide (now it requires to add new
changes in the changelog)
* Generated a new GIF with the new output
* Increased the version to `0.2.0`

I want to implement #54 and #55 before actually creating a release so
the CHANGELOG will change. But those changes shouldn't affect the
documentation so this one can be merged separately.

### Additional tasks

- [x] Documentation for changes provided/changed
- [ ] Tests added

Co-authored-by: Zixuan Zhang <[email protected]>
@chshersh chshersh self-assigned this Sep 20, 2022
chshersh added a commit that referenced this issue Sep 20, 2022
Resolves #55

### Additional tasks

- [ ] Documentation for changes provided/changed
- [x] Tests added

Co-authored-by: Mohammad Mohammadi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant