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.

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

Exempt content folder from review requirements? #136

Open
LukasWallrich opened this issue Jul 10, 2024 · 7 comments
Open

Exempt content folder from review requirements? #136

LukasWallrich opened this issue Jul 10, 2024 · 7 comments
Labels
not urgent Lower priority stale Marked by labeler

Comments

@LukasWallrich
Copy link
Contributor

Ideally, project leads would be able to make minor content changes without waiting for approval - could the content folder be exempted from the requirement to have review by someone else?

If that does not work, maybe it should live in another repository (so that we can also have fewer people able to edit the more technical parts of the webpage)?

@LukasWallrich LukasWallrich added the not urgent Lower priority label Jul 10, 2024
@DAKiersz
Copy link
Contributor

@bethaniley
Copy link
Contributor

bethaniley commented Jul 13, 2024

Hi Lukas! Team leads should absolutely have control over content changes. Although perhaps keeping one review might still be helpful for identifying typos etc.?

I explained the reason for two reviewers in another issue previously, here's what I said:

The reason we changed this to two reviewers was to prevent changes which break the website. A lot of previous approvals have been "looks good to me" approvals, in the sense that the person reviewing doesn't seem to check the code, or even outright admits that they don't know whether the untested code will work. As part of the IOI grant, we reviewed this and identified it as a potential weak point for future releases.

The vision for the two-person review system is that there would be:

  1. One technical reviewer - someone who understands the code and can actually check it. That is generally performed by myself, Dominik K., Sam G., Richard D., or Lukas W.
  2. One conceptual reviewer - does the proposed change make sense for FORRT's mission/vision? This generally comes from Flavio A., but can be done by anyone in FORRT's management.

You can think of this as analogous to peer review of a journal article. If they don't understand the conceptual framing and methodology, are they actually reviewing the paper?

TL;DR: it's primarily for safeguarding technical changes, but does seem largely unnecessary for content changes.

That being said, we need to clearly identify what's a content change and what isn't. For example, the recent Glossary changes were within the /content/ directory, but were technical changes (in the sense of changes to scripts which did not deploy as expected).

Potentially we could change the settings so that changes to markdown and image files only need one review, whereas changes to other file types need two. Looks like Dominik has identified a method to do that!

(Note: Edited for clarity & to merge two identical issues)

@DAKiersz DAKiersz self-assigned this Jul 20, 2024
@bethaniley bethaniley added the IOI grant This issue is part of the Invest in Open Infrastructure grant for Team Website label Jul 25, 2024
@LukasWallrich
Copy link
Contributor Author

I like the idea to separate this by file type rather than folder (as that also makes it easier to add static files)

@DAKiersz DAKiersz linked a pull request Jul 28, 2024 that will close this issue
@DAKiersz DAKiersz moved this from Ready to In review in Team Website's Project Board Jul 28, 2024
@DAKiersz DAKiersz assigned DAKiersz and unassigned DAKiersz Jul 28, 2024
Copy link

This issue has been inactive for more than 90 days. If there is no further activity, it will be automatically closed in seven days time. You can reopen the issue if it is still relevant.

@github-actions github-actions bot added the stale Marked by labeler label Sep 30, 2024
Copy link

github-actions bot commented Oct 8, 2024

This issue has been automatically closed due to it being stale for more than 7 days. Please feel free to reopen if you still want this issue.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2024
@github-project-automation github-project-automation bot moved this from In review to Done in Team Website's Project Board Oct 8, 2024
@LukasWallrich LukasWallrich reopened this Oct 8, 2024
@github-actions github-actions bot removed the stale Marked by labeler label Oct 9, 2024
@LukasWallrich LukasWallrich removed the IOI grant This issue is part of the Invest in Open Infrastructure grant for Team Website label Nov 18, 2024
Copy link

This issue has been inactive for more than 90 days. If there is no further activity, it will be automatically closed in seven days time. You can reopen the issue if it is still relevant.

@github-actions github-actions bot added the stale Marked by labeler label Jan 18, 2025
@LukasWallrich
Copy link
Contributor Author

LukasWallrich commented Jan 18, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not urgent Lower priority stale Marked by labeler
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants