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

README.md

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.

README.md

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

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

README.md Show resolved Hide resolved

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

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

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

doc more opts #480

Are you sure you want to change the base?

doc more opts #480

Conversation

electriquo commented Aug 3, 2021 • edited

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

Choose a reason for hiding this comment

electriquo Aug 9, 2021

Choose a reason for hiding this comment

travi Aug 9, 2021

Choose a reason for hiding this comment

electriquo Aug 9, 2021

Choose a reason for hiding this comment

johnmartel Aug 24, 2021 • edited

Choose a reason for hiding this comment

travi Aug 9, 2021

Choose a reason for hiding this comment

electriquo Aug 9, 2021 • edited

Choose a reason for hiding this comment

travi Aug 9, 2021

Choose a reason for hiding this comment

electriquo Aug 9, 2021 • edited

Choose a reason for hiding this comment

travi Aug 9, 2021

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

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