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

Update CONTRIBUTING.md #55

Merged
merged 2 commits into from
Aug 25, 2024
Merged

Update CONTRIBUTING.md #55

merged 2 commits into from
Aug 25, 2024

Conversation

Bryntet
Copy link
Contributor

@Bryntet Bryntet commented Aug 22, 2024

The message was a bit to vague before

The message was a bit to vague before
CONTRIBUTING.md Outdated
@@ -22,7 +22,7 @@ There are several ways you can contribute to Pumpkin:
If you'd like to contribute code changes, fork the Pumpkin repository on GitHub.
Install Rust at [rust-lang.org](https://www.rust-lang.org/).
Make your changes on your local fork and create a pull request to the main repository.
Ensure your code adheres to the project's coding style guidelines (if any).
Ensure your code adheres to our project structure and style guidlines.

This comment was marked as resolved.

@Bryntet
Copy link
Contributor Author

Bryntet commented Aug 23, 2024

fixed :)

@StripedMonkey
Copy link
Contributor

You say "The message was a bit too vague before", this does not address that. Right now "Our project structure and style guidelines" isn't actually defined in this document (and only structure is defined elsewhere, in STRUCTURE.md). If someone were to read this, it's completely vapid because there is nothing meaningful being said here besides "Follow my rules, if you don't know what those rules are, too bad."

A better fix, that would make this less vague, is to link to STRUCTURE.md, and explicitly say, for example, that rustfmt must be run as a first pass at "our" style guidelines.

@Snowiiii Snowiiii merged commit cb32cd3 into Pumpkin-MC:master Aug 25, 2024
5 checks passed
@Bryntet Bryntet deleted the patch-1 branch August 28, 2024 13:31
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.

4 participants