doc more opts #480

electriquo · 2021-08-03T05:48:13Z

Resolves #439
Relates #220 (comment)
Resolves #220
Resolves #420
Resolves #458
Resolves #359
Resolves #402

Closes #439 Relates #220 (comment) Resolves #220 Resolves #420 Resolves #458

travi · 2021-08-03T15:42:49Z

README.md

+  archived: false
+
+  # Can be `public`, `private` or `internal`
+  visibility: public


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

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):

https://developer.github.com/changes/2019-12-03-internal-visibility-changes/

https://docs.github.com/en/rest/reference/repos#create-an-organization-repository--parameters

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.

travi · 2021-08-03T15:45:36Z

this documents a bit more than i was expecting from #458. i'm all for documenting each of these, but want to confirm a couple of things before we move forward.

have all of these properties been confirmed to work through the settings app in addition to the documentation?
are any of them required? i see that you have mentioned how to disable if they were previously enabled, but if some need to be set to null or similar in order to avoid enabling them in the first place due to being required, we should note details like that as well

electriquo · 2021-08-03T22:02:59Z

have all of these properties been confirmed to work through the settings app in addition to the documentation?

yes

are any of them required?

no, otherwise i would have reported an issue :)

i see that you have mentioned how to disable if they were previously enabled, but if some need to be set to null or similar in order to avoid enabling them in the first place due to being required, we should note details like that as well

from many playing around, they are not required and one who does not use these options\keys will have the default github settings of the repository (again, such settings might be inherited from the github organization policy)

electriquo · 2021-08-04T19:10:56Z

Hi @travi, did I answer all of your questions? Is there a need for a further clarification?

electriquo · 2021-08-07T06:51:57Z

@travi, any feedback?

travi · 2021-08-07T20:05:06Z

Please be patient. This is still on my list. I just haven't gotten back to it yet.

travi · 2021-08-09T04:02:55Z

README.md

+  archived: false
+
+  # Can be `public`, `private` or `internal`
+  visibility: public


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.

travi · 2021-08-09T04:05:53Z

README.md

@@ -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


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?

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.

travi · 2021-08-09T04:14:27Z

README.md

@@ -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.
+      required_signatures: true


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?

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.

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

travi · 2021-08-09T04:15:03Z

README.md

@@ -155,6 +169,12 @@ branches:
        apps: []
        users: []
        teams: []
+      # Permits force pushes for all users with push access. Set to null to disable.
+      allow_force_pushes:


no value is provided here

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

travi · 2021-08-09T04:15:26Z

README.md

@@ -155,6 +169,12 @@ branches:
        apps: []
        users: []
        teams: []
+      # Permits force pushes for all users with push access. Set to null to disable.


i dont think "disable" makes sense for this one

disable is used a lot.

to which world would you like to change it?

travi · 2021-08-09T04:15:50Z

README.md

+      # Permits force pushes for all users with push access. Set to null to disable.
+      allow_force_pushes:
+      # Allows users with push access to delete matching branches. Set to false to disable.
+      allow_deletions: false


disable doesnt really make sense for this one either

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
...

README.md

electriquo · 2021-08-15T05:01:34Z

@travi: Shall we give this PR another push for the sake of getting it done before it becomes stale?

electriquo · 2021-08-20T05:52:47Z

@travi

Your work, knowledge sharing and dedication are very appreciated, especially when you do that for free.

If you need a hand in maintaining Probot Setting project, I am offering myself as your apprentice - maybe I could assist you in progressing things you like less or things in a lower priority, so the community could earn back your precious time.

travi · 2021-09-03T15:30:30Z

apologies for continued delay here. the last few weeks have been especially busy for me.

i havent been entirely ignoring you, though. the comments that i had above got me thinking further about this project's documentation that i've been considering for a while. basically, i think we need to avoid duplicating documentation that already exists for the api. instead, we should make it more clear that the api documentation is an appropriate reference for the details that can be managed by the settings app. that will help us avoid wording that doesnt quite align with the details outlined by the github technical writing staff, but will also help us avoid the situation where new properties are available from the api that we havent yet added to our examples.

i've started working on some updates to our docs that i think will help make this more clear. examples will still be provided, but it will be more clear that they are not going to include all properties or full descriptions of all that we do include.

that said, i think there are two options for this pr. either we can simplify to the single property that was asked for in #458 so that we can avoid resolving some of the other questions for now, or we can wait until after the restructuring that i'm working on before we decide which additional details would provide value beyond referencing the api documentation.

electriquo · 2021-09-04T12:58:10Z

Thank you @travi for sharing your thoughts.
You're the leader, so let's follow whatever you think is best... (I just didn't get what do you prefer from the alternative you shared)

we should make it more clear that the api documentation is an appropriate reference for the details that can be managed by the settings app

I think we have it...

https://github.com/probot/settings/blob/2a2e9333ed68c8dc9962add9d9b9228d7f2c7821/README.md#L19

electriquo · 2022-04-21T07:14:05Z

@travi i think we should try to merge this pull-request since it solves many "issues". do you think we should try to merge or close the pull-request?

electriquo · 2022-06-06T15:42:27Z

@travi would you like to merge these changes or would you like to close the pull-request?

justinmchase · 2023-02-17T01:33:58Z

Trying to run this app in enterprise cloud fails by default with the error:

Public repositories are not permitted for Enterprise Managed Organizations.

The setting needed to fix it is not documented, except in this PR. Please for the love of God merge this PR already.

nitrocode · 2023-04-19T19:24:58Z

Friendly ping @travi

travi · 2024-03-24T15:02:04Z

i'm trying to consider what to suggest for this PR after #893. at minimum, this will need to be adjusted to the updated format of the configuration documentation if we decide to include these updates. however, before investing more effort, lets come to agreement on the below thoughts:

i'm struggling with the value of continuing to add properties to the example. new properties are constantly being added to the api, so being complete in the examples is a losing battle. plus, the examples are meant to be examples with a reference to where to get the complete, official details. i dont intend for the examples to be complete.

i do appreciate the time you've invested in this and i do see value in highlighting certain parts, especially where there might be some confusion that isn't well clarified by the official docs. however, i could use some help deciding what is valuable to include here that isnt just taking the docs closer to completeness rather than examples with official docs references

electriquo · 2024-03-25T08:18:34Z

#893 is a bliss. this pr came after browsing the issues and seeing lots of repeated questions about the configuration (since most don't rtfm), which leads to comments such as #567 (comment).

i think we can close this pr. if you think otherwise, please do reopen.

doc more opts

4250d82

Closes #439 Relates #220 (comment) Resolves #220 Resolves #420 Resolves #458

electriquo marked this pull request as ready for review August 3, 2021 05:48

electriquo mentioned this pull request Aug 3, 2021

Require Conversation Resolution Before Merging Protection Rule #458

Open

travi approved these changes Aug 3, 2021

View reviewed changes

travi reviewed Aug 3, 2021

View reviewed changes

travi requested changes Aug 9, 2021

View reviewed changes

electriquo commented Aug 9, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

c7c9bdc

electriquo commented Aug 9, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

4a4ade2

electriquo commented Aug 9, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

2e85db1

electriquo commented Aug 9, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

80e9132

Update README.md

940a02f

electriquo requested a review from travi August 30, 2021 17:03

unfurl-links bot mentioned this pull request Apr 18, 2022

Repository visibility settings #359

Closed

electriquo mentioned this pull request Mar 23, 2023

Added autolinks #545

Open

electriquo mentioned this pull request Mar 24, 2024

Add support for new GitHub feature #567

Closed

1 task

electriquo closed this Mar 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc more opts #480

doc more opts #480

electriquo commented Aug 3, 2021 •

edited

travi Aug 3, 2021

electriquo Aug 3, 2021

travi Aug 9, 2021

travi commented Aug 3, 2021

electriquo commented Aug 3, 2021

electriquo commented Aug 4, 2021

electriquo commented Aug 7, 2021

travi commented Aug 7, 2021

travi Aug 9, 2021

travi Aug 9, 2021

electriquo Aug 9, 2021

travi Aug 9, 2021

electriquo Aug 9, 2021

johnmartel Aug 24, 2021 •

edited

travi Aug 9, 2021

electriquo Aug 9, 2021 •

edited

travi Aug 9, 2021

electriquo Aug 9, 2021 •

edited

travi Aug 9, 2021

electriquo Aug 9, 2021 •

edited

electriquo commented Aug 15, 2021 •

edited

electriquo commented Aug 20, 2021

travi commented Sep 3, 2021

electriquo commented Sep 4, 2021 •

edited

electriquo commented Apr 21, 2022

electriquo commented Jun 6, 2022

justinmchase commented Feb 17, 2023

nitrocode commented Apr 19, 2023

travi commented Mar 24, 2024

electriquo commented Mar 25, 2024

doc more opts #480

doc more opts #480

Conversation

electriquo commented Aug 3, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travi commented Aug 3, 2021

electriquo commented Aug 3, 2021

electriquo commented Aug 4, 2021

electriquo commented Aug 7, 2021

travi commented Aug 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnmartel Aug 24, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

electriquo Aug 9, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

electriquo Aug 9, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

electriquo Aug 9, 2021 • edited

Choose a reason for hiding this comment

electriquo commented Aug 15, 2021 • edited

electriquo commented Aug 20, 2021

travi commented Sep 3, 2021

electriquo commented Sep 4, 2021 • edited

electriquo commented Apr 21, 2022

electriquo commented Jun 6, 2022

justinmchase commented Feb 17, 2023

nitrocode commented Apr 19, 2023

travi commented Mar 24, 2024

electriquo commented Mar 25, 2024

electriquo commented Aug 3, 2021 •

edited

johnmartel Aug 24, 2021 •

edited

electriquo Aug 9, 2021 •

edited

electriquo Aug 9, 2021 •

edited

electriquo Aug 9, 2021 •

edited

electriquo commented Aug 15, 2021 •

edited

electriquo commented Sep 4, 2021 •

edited