Skip to content

Conversation

@BagToad
Copy link
Member

@BagToad BagToad commented Oct 6, 2024

This PR resolves #8946 according to the interface described in #8946 (comment)

Examples

gh repo license list

Details

image

gh repo license view <license>

Details

image

gh repo gitignore list

Details

image

Note: truncated screenshot because the list is long.

gh repo gitignore view <gitignore-template>

Details

image

Note: truncated screenshot because the output is long.

Notes

  • This PR moves and exports existing methods listLicenseTemplates and listGitIgnoreTemplates found here:
    // listGitIgnoreTemplates uses API v3 here because gitignore template isn't supported by GraphQL yet.
    func listGitIgnoreTemplates(httpClient *http.Client, hostname string) ([]string, error) {
    var gitIgnoreTemplates []string
    client := api.NewClientFromHTTP(httpClient)
    err := client.REST(hostname, "GET", "gitignore/templates", nil, &gitIgnoreTemplates)
    if err != nil {
    return []string{}, err
    }
    return gitIgnoreTemplates, nil
    }
    // listLicenseTemplates uses API v3 here because license template isn't supported by GraphQL yet.
    func listLicenseTemplates(httpClient *http.Client, hostname string) ([]api.License, error) {
    var licenseTemplates []api.License
    client := api.NewClientFromHTTP(httpClient)
    err := client.REST(hostname, "GET", "licenses", nil, &licenseTemplates)
    if err != nil {
    return nil, err
    }
    return licenseTemplates, nil
    }
    to queries_repo.go. I felt this made the most sense since gh repo create uses these methods as well.
  • I chose to go with table output for the list commands rather than just dumping out a list. Even thought gitignore list is a single column, I felt it would make sense to use the same tableprinter anyway.
  • You may notice that this PR renames shared to format in pkg/cmd/repo/create/create.go - this is because I had originally missed that there were many queries in queries_repo.go; instead of using queries_repo.go, I had implemented queries in a package under shared, which prompted me to split shared into format and queries to better organize the code. However, I later removed queries when I realized about queries_repo.go, but decided to leave format as a changed package to align more closely with the package structure found in some other examples like https://github.com/cli/cli/tree/d645fd4f0066d96b1e5ef37089192f0d33ba9b84/pkg/cmd/project/shared/format This was reversed.

Feedback

I would love constructive feedback on anything, but I am particularly interested in:

  • Did I miss any testing opportunities?
  • Comments about the mild restructure and migration of listing functions
  • Do you agree/disagree with the rename of the shared package to format? See Notes section above for more info.
    This was reversed.

@BagToad BagToad requested a review from a team as a code owner October 6, 2024 19:55
@BagToad BagToad requested a review from williammartin October 6, 2024 19:55
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Oct 6, 2024
Copy link
Member

@andyfeller andyfeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure gh users will be happy for this information readily at hand! ✨

No major concerns but a range of various minor or middling nits. 🙇

BagToad and others added 2 commits October 8, 2024 14:00
@BagToad BagToad force-pushed the bagtoad/cli-8946-list-licenses-and-gitignore-options branch from 79b617a to c2a7756 Compare October 10, 2024 02:49
Licenses are no longer referred to as templates. Prefix the new license and gitignore functions with "Repo" as a more descriptive name.
@BagToad BagToad force-pushed the bagtoad/cli-8946-list-licenses-and-gitignore-options branch from fe152bc to d6c6c6b Compare October 10, 2024 03:15
Updated `Use` to indicate that args can be an SPDX ID or license key.
Improved the examples docs to show the use of both.
@BagToad
Copy link
Member Author

BagToad commented Oct 10, 2024

@andyfeller This is ready for another pass please 🙏 I've implemented most of your suggestions. There are a few unresolved comments that need more discussion.

@BagToad BagToad requested a review from andyfeller October 10, 2024 04:43
Copy link
Member

@andyfeller andyfeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final round of minor nits, primarily around language. Happy to knock this out with you today if you like.

Copy link
Member

@andyfeller andyfeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ 👨‍🍳 💋

@andyfeller andyfeller enabled auto-merge October 11, 2024 18:28
@andyfeller andyfeller merged commit b91dab4 into trunk Oct 11, 2024
@andyfeller andyfeller deleted the bagtoad/cli-8946-list-licenses-and-gitignore-options branch October 11, 2024 18:29
Accuweaty24 added a commit to Accuweaty24/cli that referenced this pull request Oct 11, 2024
Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
@probablycorey @matschaffer @digitalfu cli#9745 cli#9721 cli#9728 cli#9746 №accuweaty24 #
@yermulnik
Copy link

yermulnik commented Oct 16, 2024

User feedback FWIW: reading v2.59.0 What's Changed section I noticed nifty repo gitignore list/view item, which I thought was about repo's .gitignore file(s). Though apparently looking into this PR I regretfully found this feature is about gitignore templates 🤷🏻 Would be great to eliminate this confusion by replacing gitignore sub-command with e.g. gitignore-templates or similar. Thanks.
Same probably applies to license sub-command too.

@yermulnik
Copy link

Would be great to eliminate this confusion by replacing gitignore sub-command with e.g. gitignore-templates or similar. Thanks.
Same probably applies to license sub-command too.

Or maybe even by adding template sub-command with nested license and gitignore sub-commands 🤔

@andyfeller
Copy link
Member

andyfeller commented Oct 16, 2024

User feedback FWIW: reading v2.59.0 What's Changed section I noticed nifty repo gitignore list/view item, which I thought was about repo's .gitignore file(s). Though apparently looking into this PR I regretfully found this feature is about gitignore templates 🤷🏻 Would be great to eliminate this confusion by replacing gitignore sub-command with e.g. gitignore-templates or similar. Thanks. Same probably applies to license sub-command too.

@yermulnik : I'd love to follow up within the original issue or a new issue about your ideas on what gh might do with existing .gitignore files within the repository.

Normally, I'd say gh is focused on helping people focus on the primitives and information across GitHub while leaving git for users working with the contents within the repository. The original issue here was helping developers creating repositories understand what various licenses and .gitignore templates entailed and generating the initial content.

@yermulnik
Copy link

@yermulnik : I'd love to follow up within the original issue or a new issue about your ideas on what gh might do with existing .gitignore files within the repository.

Apologies if I wasn't clear enough: this is not about handling .gitignore files, but rather this is about misleading sub-command naming.

[…] various licenses and .gitignore templates […]

This was the gist of my comment: the word "templates", that you used to make thought clear, is missing from the added sub-commands and this adds a bit of confusion I guess.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Oct 17, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cli/cli](https://github.com/cli/cli) | minor | `v2.58.0` -> `v2.59.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>cli/cli (cli/cli)</summary>

### [`v2.59.0`](https://github.com/cli/cli/releases/tag/v2.59.0): GitHub CLI 2.59.0

[Compare Source](cli/cli@v2.58.0...v2.59.0)

#### What's Changed

-   Allow community submitted design work by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#9683
-   Improve `SECURITY.md` with expectations for privately reported vulnerabilities by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#9687
-   Emit a log message when extension installation falls back to a `darwin-amd64` binary on an Apple Silicon macOS device by [@&#8203;timrogers](https://github.com/timrogers) in cli/cli#9650
-   Print the login URL even when opening a browser by [@&#8203;ulfjack](https://github.com/ulfjack) in cli/cli#7091
-   configurable maxwidth for markdown WithWrap() by [@&#8203;smemsh](https://github.com/smemsh) in cli/cli#9626
-   Handle errors when parsing hostname in auth flow by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#9729
-   Add `repo license list/view` and `repo gitignore list/view` by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#9721
-   Introduce testscript acceptance tests generally, and for the MR command specifically by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#9745
-   Support `GH_ACCEPTANCE_SCRIPT` env var to target a single script by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#9756
-   Ensure Acceptance defer failures are debuggable by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#9754
-   Add acceptance task to makefile by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#9748
-   Add Acceptance tests for `issue` command by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#9757
-   Update IsEnterprise and IsTenancy for orthogonality using go-gh by [@&#8203;jtmcg](https://github.com/jtmcg) in cli/cli#9755
-   Supporting filtering on `gist list` by [@&#8203;heaths](https://github.com/heaths) in cli/cli#9728

#### New Contributors

-   [@&#8203;ulfjack](https://github.com/ulfjack) made their first contribution in cli/cli#7091
-   [@&#8203;smemsh](https://github.com/smemsh) made their first contribution in cli/cli#9626

**Full Changelog**: cli/cli@v2.58.0...v2.59.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external pull request originating outside of the CLI core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't see a clear way to show all available options for gh repo create <name> -l <license>

5 participants