-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: add Sean Sanker Jr to /about
page
#299
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
b26fe25
to
9a96027
Compare
9a96027
to
1b11f2b
Compare
@@ -20,6 +20,7 @@ | |||
"Jérémie", | |||
"knip", | |||
"Mariah", | |||
"Sanker", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, it looks like your branch is not up-do-date with main
(rebase onto main
), so you're missing a word that's been added since. Also if you could put packagejson
above for an alphabetical sort, that'd be awesome 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You got it! Rebased and sorted!
Questions:
- Is there some local tooling that should be installed / run to maintain the alphabetical order or is this just something that's manually maintained? I see
format
links toprettier .
but that doesn't perform sorting. - Are there any other conventions I should be aware of? Where would I find those?
- Regarding the PR template and the
compliance
step: Should I revise my PR description to exactly match the template? Is it better to add back the checkboxes but leave them empty, or omit them entirely (even if it results in a failed GH action)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- That's a great question! We use https://github.com/azat-io/eslint-plugin-perfectionist which helps with this a ton, but in the case of arrays* it is tricky because in many cases
[1, 2]
and[2, 1]
can mean very different things. So I'm afraid in this case we can't easily alphabetically order them, unless I'm not aware of something (in which case I would love to be!). - I am still learning this as I recently started taking care (owning?) the website repo. I think running the lint command will get you most of the way there (and the rest goes through GitHub checks, though I'm not a fan of GitHub checks that cannot be run locally).
- I'm of the personal opinion that this compliance step does not provide much value while acting as a deterrent to open source contribution†. Same goes for semantic commits or the "unresolved conversations" check. I know @JoshuaKGoldberg gave me ⚡ power ⚡ to change those, I just don't like to change existing/established rules policies simply based on my own personal preferences, and want more community opinions and/or actual arguments (outside of my opinions) before making the change :)
* The one exception is https://perfectionist.dev/rules/sort-array-includes where [1, 2].includes(1)
and [2, 1].includes(1)
mean the same thing, but that is very rare...
† Years ago, @darobin wrote something (I think it was a blog post?) noting that conventional commit messages were just one more step to the already long list of checks an open source contributor has to worry about and that a simple typo fix made in an online code editor could turn into a whole complicated thing. I don't know if that's still his opinion, but over the years this has resonated with me so many times, I just want things to be simple for people who want to help. Robin, if you happen to remember where that article lives, I would love to have another read :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed response! I look forward to collaborating with you! ✨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same goes for semantic commits or the "unresolved conversations" check. I know @JoshuaKGoldberg gave me ⚡ power ⚡ to change those, I just don't like to change existing/established rules policies simply based on my own personal preferences
FWIW I'm already strongly in favor of removing the compliance action 😄. It comes from my https://github.com/JoshuaKGoldberg/create-typescript-app starter that's geared more towards worked-on-by-the-community open source published packages. This site is much more team-managed. A lot of little tasks are going to keep coming up where adding a backing issue would be annoying overhead. I don't think the action is super valuable for this repo.
1b11f2b
to
e04f238
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome on board! :D
@all-contributors please add @SeanSanker for code.
|
I've put up a pull request to add @SeanSanker! 🎉 I couldn't determine any contributions to add, did you specify any contributions? |
Adds @SeanSanker as a contributor for code. This was requested by JoshuaKGoldberg [in this comment](#299 (comment)) --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
PR Checklist
Overview
🛠️