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

Add CONTRIBUTING.md #61

Merged
merged 1 commit into from
Nov 26, 2023
Merged

Add CONTRIBUTING.md #61

merged 1 commit into from
Nov 26, 2023

Conversation

coavins
Copy link
Owner

@coavins coavins commented Nov 26, 2023

No description provided.

@coavins coavins self-assigned this Nov 26, 2023
@coavins
Copy link
Owner Author

coavins commented Nov 26, 2023

@macinsight Can you review? Any thoughts/suggestions?

Copy link
Collaborator

@macinsight macinsight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good guidelines on how to contribute.

I'd swap the bit about luacheck mentioning GH Action and manually running, just to mentally drive the point home that it runs on commit/PR-merge, because right now it (at first read) it reads like you're expected to run luacheck yourself.
Current wording:

We use luacheck for linting, just run luacheck src in the project root.
Github will automatically run this check on your behalf when submitting a pull request.

Suggested change:

We use luacheck for linting. GitHub automatically runs it as a GitHub Action when you commit or open a Pull Request against the develop branch.
If you wish, you can lint your additions yourself before pushing them to the repository by running luacheck src in the repository root.

Apart from that, I'm quite happy with it. Concise, welcoming, ticks all the boxes.

@macinsight
Copy link
Collaborator

image

I may be mistaken, but didn't I just review this? Even approved it 'cause I wasn't entirely happy with my own suggested changes after reading it three times.

@macinsight
Copy link
Collaborator

macinsight commented Nov 26, 2023

re my last, per the docs:

If a collaborator with admin, owner, or write access to the repository submits a review requesting changes, the pull request cannot be merged until the same collaborator submits another review approving the changes in the pull request.

That might just be why. Unintuitive, to say the least.

Edit: Naaah, you're repo-owner, not collab. Technically. Eh, nevermind, my meds ain't kicked in yet.

@coavins
Copy link
Owner Author

coavins commented Nov 26, 2023

I'm trying to figure out how the permissions work, it's all very different than what I'm used to (Azure Devops). Once I have it sorted out, you should be able to review and merge PRs for the develop branch.

@coavins coavins merged commit 2f5c6b1 into develop Nov 26, 2023
1 check passed
@coavins coavins deleted the add-contrib branch November 26, 2023 22:14
@macinsight
Copy link
Collaborator

Oh god, Azure. I've been 100% Linux for about a decade now, never worked with Azure, hell, haven’t even touched a MS system in the last five years, minus old, crusty Windows 2003 and earlier Servers required for mission-critical traffic light control operations. Those are still working, bare-metal, too!

anyway!
You should be able to go into “Repository Settings”, then “Collaborators”, then add me there. That should grant me that power.

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.

2 participants