Add public field to repository pages#2540
Add public field to repository pages#2540BartolottiLuca wants to merge 22 commits intointegrations:mainfrom
Conversation
Co-authored-by: Usman <akeju00+github@gmail.com>
|
I'd really like to have this feature available! Please! |
github-project-automation
bot
moved this from 👀 In review
to 🏗 In progress
in 🧰 Octokit Active
|
@BartolottiLuca Please rebase your PR :) |
better field description Co-authored-by: Timo Sand <timo.sand@iki.fi>
|
@deiga Thanks for the review, I've applied your suggestions and rebased |
| t.Run("with an anonymous account", func(t *testing.T) { | ||
| t.Skip("anonymous account not supported for this feature") | ||
| }) | ||
|
|
||
| t.Run("with an individual account", func(t *testing.T) { | ||
| t.Skip("individual account not supported for this feature") | ||
| }) | ||
|
|
||
| t.Run("with an organization account", func(t *testing.T) { | ||
| testCase(t, organization) | ||
| }) |
There was a problem hiding this comment.
we don't use this structure anymore, please follow the new convention :)
There was a problem hiding this comment.
gave the new convention a go @deiga. let me know if it's correct
| }, | ||
| "public": { | ||
| Type: schema.TypeBool, | ||
| Computed: true, |
There was a problem hiding this comment.
question: This doesn't seem to get set anywhere and in the docs you write that it's optional. Did you mean to use Optional here? Should it have Default: false as well to remove the need for type-checking this later?
There was a problem hiding this comment.
I'm unsure which is the best option here. From the docs
To publish a GitHub Pages site privately, your organization must use GitHub Enterprise Cloud. For more information about how you can try GitHub Enterprise Cloud for free, see Setting up a trial of GitHub Enterprise Cloud.
If your enterprise uses Enterprise Managed Users, GitHub Pages sites can only be published as private, and all GitHub Pages sites are only accessible to other enterprise members. For more information about Enterprise Managed Users, see GitHub Pages limits.
If your organization uses GitHub Enterprise Cloud without Enterprise Managed Users, you can choose to publish your project sites privately or publicly to anyone on the internet.
So there's no clear default since:
- anyone without github enterprise will be forced to have public pages
- If you have enterprise and Enterprise Managed Users, the pages can only be private
- only users that have enterprise and not Enterprise Managed Users can choose between public and private
I guess Computed: true is safer?
There was a problem hiding this comment.
Yeah, that does look problematic. I think that having only Computed: true prevents users from setting the value, which would be unwelcome for GHEC non-EMU cases.
Maybe you could check if the SDK sends the field or it's left blank, otherwise the Create and Update need to ensure it's not being sent unless explicitly set.
You're right that Default isn't a good idea, but I guess it needs to be Computed & Optional
Co-authored-by: Timo Sand <timo.sand@iki.fi>
Co-authored-by: Timo Sand <timo.sand@iki.fi>
|
|
||
| * `cname` - (Optional) The custom domain for the repository. This can only be set after the repository has been created. | ||
|
|
||
| * `public` - (Optional) Whether the GitHub Pages site is publicly visible. If set to `true`, the site is accessible to anyone on the internet. If set to `false`, the site will only be accessible to users who have at least `read` access to the repository that published the site. |
There was a problem hiding this comment.
Could you add a link to the section in the GH docs about the limitations for different Owner types? I think that would be valuable info for users
| ) | ||
|
|
||
| resource.Test(t, resource.TestCase{ | ||
| PreCheck: func() { skipUnlessMode(t, organization) }, |
There was a problem hiding this comment.
nitpick: we have a skipUnlessHasOrgs (or similar) function
|
@deiga sorry it took me so long, I've applied all the suggestions |
4 participants
Resolves #1045
Before the change?
After the change?
publicfieldPull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!