OpenWebAdvocacy / website

Source code for the Open Web Advocacy website.
https://open-web-advocacy.org/
MIT License
33 stars 24 forks source link

website repo setup #4

Open thescientist13 opened 2 years ago

thescientist13 commented 2 years ago

Coming out of the discussions in Discord and piggybacking off of #1 , wanted to start a discussion for some repo setup and settings that I could help out with.

  1. [x] CONTIBRUTING.md
  2. [x] LICENSE file (and what license?)
  3. [x] Code of Conduct (just use contributors covenant?)
  4. [x] .editorconfig
  5. [x] Issue and PR templates (any preferred templates to suggest?)
  6. [x] engines field in package.json for Node / npm (what minimum version of Node should we support?)
  7. [ ] Netlify hosting and Deploy Previews - https://github.com/OpenWebAdvocacy/website/pull/12
    • using the Open Source Plan? (requires CoC, License, and netlify link / logo on the home page)
  8. [ ] Branch protections and / or Codeowners (who should have final merge rights?)
  9. [ ] GitHub Actions (Netlify build could be enough here depending on what our needs are for CI / CD)

Let me know if there's anything else I missed and should add. Might need some help for some of the integrations re: GitHub repo access and hosting configurations, but I can call that out in the PR at that time.

aliblackwell commented 2 years ago

Great thinking @thescientist13.

Does engines swap your version of Node to the correct version? I have recently come across the use of .nvmrc to specify Node version, and if you use NVM to manage your node version and cd into a project with .nvmrc, your Node version changes automatically.

I had an issue on another project this morning where I was on Node 17, I didn't have a .nvmrc and the project didn't work with latest node, I had to use 16. Based on this experience, I'd suggest we use latest v16, which is 16.14.0. v17 is quite bleeding edge!

In terms of minimum version, I'd suggest expecting all contributors to use the same version, and where people don't for whatever reason, any updates to package-lock.json that get committed to main branch are made by a Codeowner using the correct version - this should help prevent snafus.

On .editorconfig, there is a .prettierrc in the project already. Setting up husky to run lint-staged as a pre-commit hook can be a bit of a faff. I have done it on my JAMStack Starter but there must be a better way... It's obviously important to have shared config... do we just have it as part of code review that things need to be formatted correctly, based on .editorconfig? How would you see it working with .editorconfig?

Also indentation... two spaces instead of tabs!? That's my preference, but I know it can be a divisive issue!

In terms of branch protection, I'd suggest all PRs are based off a branch called staging and everything gets code reviewed going into it, and commits trigger a deploy on Netlify. The main branch is rebased into by Codeowners to keep the commit history tidy, and any commit into main triggers a production deploy.

That is my ten pence! Thanks for taking the lead on this :)

thescientist13 commented 2 years ago

Thanks for the feedback @aliblackwell !

Does engines swap your version of Node to the correct version? I have recently come across the use of .nvmrc to specify Node version, and if you use NVM to manage your node version and cd into a project with .nvmrc, your Node version changes automatically.

engines does not do that, it will just bail when running npm install but am happy to add a .nvmrc file to the project as well. With in place, it should hopefully guide all contributors down the right path if they do / don't have have NVM.

I had an issue on another project this morning where I was on Node 17, I didn't have a .nvmrc and the project didn't work with latest node, I had to use 16. Based on this experience, I'd suggest we use latest v16, which is 16.14.0. v17 is quite bleeding edge!

Agreed, will set it to latest LTS version at time of the PR.

In terms of minimum version, I'd suggest expecting all contributors to use the same version, and where people don't for whatever reason, any updates to package-lock.json that get committed to main branch are made by a Codeowner using the correct version - this should help prevent snafus.

Yeah, for that reason i suggest (to myself) to updating the README steps to favor npm ci instead of npm install so that way dependency changes only happen when expected.

On .editorconfig, there is a .prettierrc in the project already. Setting up husky to run lint-staged as a pre-commit hook can be a bit of a faff. I have done it on my JAMStack Starter but there must be a better way... It's obviously important to have shared config... do we just have it as part of code review that things need to be formatted correctly, based on .editorconfig? How would you see it working with .editorconfig?

I'm somewhat leary of pre-commit hooks and don't really jam with Prettier much just from experience, but certainly happy to follow the conventions of the project. Editorconfig is a small set of formatting options and so prettier would certainly allow for a more robust set of opinions on the matter.

Also indentation... two spaces instead of tabs!? That's my preference, but I know it can be a divisive issue!

I have no strong feeling on the matter other than consistency.

In terms of branch protection, I'd suggest all PRs are based off a branch called staging and everything gets code reviewed going into it, and commits trigger a deploy on Netlify. The main branch is rebased into by Codeowners to keep the commit history tidy, and any commit into main triggers a production deploy.

Any reason to not just deploy straight from main? With the deploy previews and branch protections + codeowners, I think the second branch could probably be eliminated entirely. That said if someone wants to maintain that subset of git flow, then I certainly won't object, but for simplicity, I think deploying on merge into main is pretty nice.

thescientist13 commented 2 years ago

@capjamesg Can we keep this open to help facilitate the conversation on the remaining CI / CD related tasks? (I don't have permissions myself)

thescientist13 commented 2 years ago

Wanted to leave some thoughts down on the last part(s) around hosting and CI / CD, etc.

So (IMO) the most straightforward approach to start with here is using Netlify and GitHub as follows for a fully automated workflow

The general workflow would look something like this

  1. Contributor submits a PR
  2. Netlify starts a build and generates a deploy preview and drops a link to into the PR
  3. (Optionally) A GitHub action runs for supplemental build steps like for linting, formatting, testing, etc and sets a status on the PR
  4. Reviewers and Code Owners review the code, test in the deploy preview environment and approve the PR
  5. Once statuses and other requirements are met, those with branch permissions can merge the PR
  6. On merge into main, Netlify will build for production and deploy a new build

Here are some examples of how I typically have Netlify and GitHub configured for all these various settings

Deploy Previews

Screen Shot 2022-03-09 at 9 41 50 PM

Branch Permissions (CI)

Screen Shot 2022-03-09 at 9 42 34 PM Screen Shot 2022-03-09 at 9 42 44 PM Screen Shot 2022-03-09 at 9 42 57 PM

Hosting (CD)

Screen Shot 2022-03-09 at 9 42 15 PM
thescientist13 commented 2 years ago

Would also want to mention we can do all of the above without impacting the current production site so we can get all the workflow benefits (previews, "production" site) while operating under the radar, so to speak. Netlify will give us a custom subdomain like silly-something.netlify.app that we can use until we are ready to officially switch over DNS from the current site to the Netlify hosted one.

This could be really helpful, especially deploy previews, for sharing / proposing site designs, information architecture ideas, etc just through PRs and the links to the deploy preview site, which is really a plus for collaboration IMO.

ssddanbrown commented 2 years ago

Hi @thescientist13, The branching and workflow stuff for netlify sounds really cool. I guess the main challenge right now would be management/ownership of the Netlify account, and accountability of any pricing that may come into play (Don't use them myself but seems like they charge for quite a lot of different areas, could get caught out there).

I'm wondering if, for now, to get things moving and to a live state, we should just throw the site up on GitHub pages on a pages/release/publish branch and use that for now, then we can always point back to Netlify to get that cool features in the future if needed.

thescientist13 commented 2 years ago

Hey @ssddanbrown I operate most of my Netlify sites on just the standard plan alone, which gives you everything above for free. I believe that unlike AWS, you can sign up without a CC and just go from there. So far the only thing I have paid for (intentionally) was Analytics for one of my projects, and have used forms for free. Not sure if there are other features we are interested in at the moment, like functions, but I can help "shop" for those as well.

There is also the Open Source Plan I mentioned, which is a free version of the Pro plan.

But yes, ultimately someone with admin permissions to this GitHub repo would have to set all this stuff up in Netlify and GitHub, but at least it is just a one time setup and pretty set it and forget it after that.

A couple options I see are, have an admin setup the Netlify account

  1. Set it up themselves
  2. Zoom / screen share session to walk the admin through the setup
  3. Grant one person limited to set everything up (a weekend) and then revoke once the workflow is established

Assuming there is no way in the door to Netlify without a CC, then yeah, we can at least setup GitHub pages with some GitHub Actions automation on merge, but I think it's at least worth having someone taking a look at Netlify and just see how far we can get for free, if not for the deploy previews alone, which I think are worth it for a content and design heavy phase of this project we are in.

Just my $.02. ✌️