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.

Sign up for GitHub

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

doc more opts #480

Closed
wants to merge 6 commits into from
Closed

doc more opts #480

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4250d82
doc more opts
Aug 3, 2021
c7c9bdc
Update README.md
electriquo Aug 9, 2021
4a4ade2
Update README.md
electriquo Aug 9, 2021
2e85db1
Update README.md
electriquo Aug 9, 2021
80e9132
Update README.md
electriquo Aug 9, 2021
940a02f
Update README.md
electriquo Aug 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ repository:
# vulnerability alerts.
enable_vulnerability_alerts: true

# Either `true` to make this repo available as a template repository or `false` to prevent it.
is_template: false
# template_repository: false
Copy link
Member

Choose a reason for hiding this comment

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

i see is_template listed on https://docs.github.com/en/rest/reference/repos#update-a-repository, but i dont find template_repository mentioned there. were both intentionally included here?

also, is there a reason that this one is commented out?

Copy link
Author

Choose a reason for hiding this comment

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

you can see it on https://docs.github.com/en/rest/reference/repos#list-organization-repositories

The is_template and template_repository keys are currently available for developer to preview.


# Note: You cannot unarchive repositories through the API. `true` to archive this repository.
archived: false

# Can be `public`, `private` or `internal`
electriquo marked this conversation as resolved.
Show resolved Hide resolved
visibility: public
Copy link
Member

Choose a reason for hiding this comment

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

does this conflict with the existing private attribute? is the use of private vs visibility mutually exclusive?

Copy link
Author

Choose a reason for hiding this comment

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

they are different and it depends whether you are github enterprise user and what is the organization (within the enterprise) policy.

e.g. the organization set the default repository visibility to private, even if one will use private: false it won't affect the organization policy. yet, you can set visibility to internal or private.

here are some references for clarity (i don't want to copy and paste):

Copy link
Member

Choose a reason for hiding this comment

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

i guess what i'm getting at is that i think we should clarify what happens when both private and visibility are defined together and maybe discourage defining both. from https://docs.github.com/en/rest/reference/repos#update-a-repository:

The visibility parameter overrides the private parameter when you use both along with the nebula-preview preview header.

and it depends whether you are github enterprise user and what is the organization (within the enterprise) policy.

i think this is the other piece that should be mentioned here since internal doesnt apply in all cases. again from https://docs.github.com/en/rest/reference/repos#update-a-repository, i like the way that is described:

Can be public or private. If your organization is associated with an enterprise account using GitHub Enterprise Cloud or GitHub Enterprise Server 2.20+, visibility can also be internal.




# Labels: define labels for Issues and Pull Requests
labels:
- name: bug
Expand Down Expand Up @@ -146,6 +158,8 @@ branches:
strict: true
# Required. The list of status checks to require in order to merge into this branch
contexts: []
# Commits pushed to matching branches must have verified signatures. Set to false to disable.
electriquo marked this conversation as resolved.
Show resolved Hide resolved
required_signatures: true
Copy link
Member

Choose a reason for hiding this comment

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

i dont find this attribute mentioned in the docs at https://docs.github.com/en/rest/reference/repos#update-branch-protection

did you find details about this one somewhere else?

Copy link
Author

Choose a reason for hiding this comment

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

found it on https://docs.github.com/en/rest/reference/repos#create-commit-signature-protection

and although i am writing it in this comment, all new addition have been tested.

Copy link

@johnmartel johnmartel Aug 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

You can also find earlier confirmation of @foolioo's statement in this comment

# Required. Enforce all configured restrictions for administrators. Set to true to enforce required status checks for repository administrators. Set to null to disable.
enforce_admins: true
# Prevent merge commits from being pushed to matching branches
Expand All @@ -155,6 +169,12 @@ branches:
apps: []
users: []
teams: []
# Permits force pushes for all users with push access. Set to null to disable.
Copy link
Member

Choose a reason for hiding this comment

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

i dont think "disable" makes sense for this one

Copy link
Author

@electriquo electriquo Aug 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

disable is used a lot.

to which world would you like to change it?

allow_force_pushes:
Copy link
Member

Choose a reason for hiding this comment

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

no value is provided here

Copy link
Author

@electriquo electriquo Aug 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

no value is considered null, same as https://github.com/foolioo/settings/blob/4250d82870301f6df716780ddc90a50a0540e98f/README.md#L141

# Allows users with push access to delete matching branches. Set to false to disable.
electriquo marked this conversation as resolved.
Show resolved Hide resolved
allow_deletions: false
Copy link
Member

Choose a reason for hiding this comment

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

disable doesnt really make sense for this one either

Copy link
Author

@electriquo electriquo Aug 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

i only kept the same terminology. although there are many places when disable was used, would you like to change "disable" to "disallow"?

https://github.com/foolioo/settings/blob/4250d82870301f6df716780ddc90a50a0540e98f/README.md#L36
https://github.com/foolioo/settings/blob/4250d82870301f6df716780ddc90a50a0540e98f/README.md#L39
https://github.com/foolioo/settings/blob/4250d82870301f6df716780ddc90a50a0540e98f/README.md#L43
https://github.com/foolioo/settings/blob/4250d82870301f6df716780ddc90a50a0540e98f/README.md#L46
https://github.com/foolioo/settings/blob/4250d82870301f6df716780ddc90a50a0540e98f/README.md#L141
...

# Ensure all review conversations are seen and addressed prior to merging
required_conversation_resolution: true
```

### Notes
Expand Down