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

Make Brief Summary By less strict #130

Closed
wants to merge 5 commits into from
Closed

Conversation

ITD88
Copy link
Contributor

@ITD88 ITD88 commented Nov 23, 2021

resolves #123
resolves #83

@kgroeneveld
Copy link
Collaborator

Ideally I don't think completely unrelated items should be included in the same PR. And ideally the commit messages should be more detailed about what they are doing and which issues it fixes.

I tested the fix for #123 in this PR and it seems OK.

I also tested the fix for #83 and it has issues. Technically it does implement what is described in #83. But when it was asked to only allow admins to to change these fields what was really meant was MCs (and admins). The MCs often refer to themselves as the admins but technically their user accounts are not admin accounts.

I just created #135 which has the same changes for #123 as this PR contains so that it can be considered separately. (Technically I also included an unrelated change, but it is at least somewhat related.)

I started working on #83 a long time ago but never finished. Now that it is fresh in memory again I hope to actually finish my change and create a new PR within the next couple days.

@notartom
Copy link
Member

@ITD88 has mentioned that they're still learning the GitHub PR workflow. I wanted to extract the useful stuff out of this PR into separate commits, a bit like I did with 5f19782 and 3d0c92f from #128, but thankfully instead of sitting forgotten in my backlog, @kgroeneveld went ahead and actually did it. We can leave this open for now, and let's close it once the "equivalent" PRs have landed.

@notartom
Copy link
Member

notartom commented Feb 6, 2022

I think with PR #137 this can be closed, right?

@notartom notartom closed this Feb 6, 2022
@kgroeneveld
Copy link
Collaborator

I think with PR #137 this can be closed, right?

Yes. All the changes in this PR have now been merged as separate commits or implemented in alternate ways. Since you already closed this PR it seems you already came to the same conclusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Brief Summary By" field parameters too strict Change edit ability in workflow profiles
3 participants