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

Cleanup contributing copy and streamline repo styling #762

Merged
merged 10 commits into from
May 7, 2024

Conversation

TylerJang27
Copy link
Collaborator

@TylerJang27 TylerJang27 commented Apr 30, 2024

  1. Updates and streamlines the copy for README.md, CONTRIBUTING.md, and tests/README.md to only focus on the important points and self-reference each other more clearly
    • Also underscores the autogen feature (requires a daemon launch)
  2. Replaces some of the repo tests with a node linter definition-checker that enforces stylistic choices in the repo and recommends conventions
    • Validates no linters/tools/actions are explicitly enabled
    • Ensures linters have description and suggest_if set
    • Ensures all linters have a relevant test file
  3. Fixes those conventions with a few places that fell through the cracks
  4. Extracts out the codegen into separate template files and reference them from CONTRIBUTING.md

Copy link

trunk-io bot commented Apr 30, 2024

⏱️ 4h 53m total CI duration on this PR
Job Cumulative Duration Recent Runs
Linter Tests ubuntu-latest 1h 43m 🟩🟩🟩 (+1 more)
Linter Tests macOS 1h 11m 🟩🟩🟩 (+1 more)
Tool Tests (ubuntu-latest) 37m 🟩🟩🟩🟩 (+1 more)
Windows Linter Tests 27m 🟩🟩🟩🟩🟩 (+1 more)
Tool Tests (macOS) 23m 🟩🟩🟩🟩🟩 (+1 more)
Trunk Check runner [linux] 10m 🟩🟩🟩🟩🟩 (+1 more)
CodeQL-Build 10m 🟩🟩🟩🟩🟩 (+1 more)
Action Tests 9m 🟩🟩🟩🟩🟩 (+1 more)
Repo Tests / Plugin Tests 3m 🟩🟩🟩🟩🟩 (+1 more)
Detect changed files 51s 🟩🟩🟩🟩🟩 (+1 more)
Aggregate Test Results 13s 🟥🟥🟩🟩🟩 (+1 more)

settingsfeedbackdocs ⋅ learn more about trunk.io

@TylerJang27 TylerJang27 marked this pull request as ready for review April 30, 2024 22:41
@TylerJang27 TylerJang27 requested a review from det April 30, 2024 22:42
@TylerJang27 TylerJang27 mentioned this pull request Apr 30, 2024
4 tasks
2. Create a directory inside `linters/` with the name of your new linter.
3. Inside this new directory, create the following structure. Most of these files will be
automatically created for you:
2. Run `mkdir linters/<my-linter>` to start. This should autopopulate with a sample

Choose a reason for hiding this comment

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

Do you have to run this before check finishes? I think this is the part I missed 😆

Copy link
Collaborator Author

@TylerJang27 TylerJang27 May 2, 2024

Choose a reason for hiding this comment

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

Really trunk check here is just a shorthand for launching our daemon, which listens for file changes in order to run Trunk Actions. trunk check should finish almost immediately if you're starting from a fresh checkout.

Running mkdir after that (>=1-2 seconds after the trunk check command starts) should be sufficient. But if you ran something like code linters/vale/plugin.yaml, the script won't autogen since it only checks empty directories :)

Copy link
Contributor

Choose a reason for hiding this comment

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

We have trunk daemon start that just stats the daemon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I prefer having trunk check here in the documentation just to normalize standard usage

@gewenyu99
Copy link

Nice :D ❤️ This would definitely be helpful for the next contributor.

I think the only other thing that took me some time to figure out was all the temporary folders generated when a tool's downloaded/when tests are running locally.

They were useful to check when things didn't act like I'd expect. Would be nice if smwhere in contributing we can point in the general direction. I happened to find them just by some weird greps I tried running.

@TylerJang27
Copy link
Collaborator Author

@gewenyu99 Thanks for the feedback! I'll update this to make sure that info is more discoverable!

2. Create a directory inside `linters/` with the name of your new linter.
3. Inside this new directory, create the following structure. Most of these files will be
automatically created for you:
2. Run `mkdir linters/<my-linter>` to start. This should autopopulate with a sample
Copy link
Contributor

Choose a reason for hiding this comment

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

We have trunk daemon start that just stats the daemon.

@TylerJang27 TylerJang27 merged commit 8ce856c into main May 7, 2024
15 checks passed
@TylerJang27 TylerJang27 deleted the tyler/contributing-revamp branch May 7, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants