Skip to content

[Merged by Bors] - CI - Check that examples are listed in README and Cargo #1650

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

Closed
wants to merge 9 commits into from
Closed

[Merged by Bors] - CI - Check that examples are listed in README and Cargo #1650

wants to merge 9 commits into from

Conversation

Weibye
Copy link
Contributor

@Weibye Weibye commented Mar 14, 2021

Closes #1581

Internal File/Link Consistency checker Action

This pull request adds an action to the CI that parses the ./examples folder for files and cross references that with the links listed in README.md and Cargo.toml to ensure the documentation actually reflects the examples currently in the repo.

The primary reason for why we want this, is to prevent people from adding new examples but forgetting to also list them in the docs, or accidentally entering broken links (typos in docs).

For details on how the action is working: Check out the README here

@Ratysz Ratysz added the A-Build-System Related to build systems or continuous integration label Mar 14, 2021
@Weibye
Copy link
Contributor Author

Weibye commented Mar 27, 2021

@alice-i-cecile @mockersf @MinerSebas
Continuing conversation from #1650 over here:

This is a screenshot of the action currently in use on this pull request: https://github.com/bevyengine/bevy/pull/1650/checks?check_run_id=2209782326

I am mostly happy with the results but I'm currently trying to improve the clarity and directness of the output so that users immediately know what corrections need to take place.

image

In this case the action has picked up 3 issues, which on closer inspection is really 2 distinct problems:

  1. ./examples/3d/pbr.rs is currently missing from README and must be added
  2. ./examples/3d/pbr_textures.rs is missing from both README and Cargo and should be added to both targets.
  3. ./examples/3d/[pbr].rs is found in README, but there has not been found an accompanying file on disk.

Now, 1 and 3 is the same issue: There is a typo in the README (brackets should not be there).

Do you all have any insight into how I can improve the outputs given?
Since this is potentially going to affect quite a few people, I want this to be as helpful and direct as possible and not cause unnecessary friction.

Edit: Is there any legitimate reason for the link ./examples/3d/[pbr].rs to have brackets in them in a Markdown document?

@Weibye
Copy link
Contributor Author

Weibye commented Apr 9, 2021

Think I really messed up my git rebasing, will try to fix

@Weibye
Copy link
Contributor Author

Weibye commented Apr 9, 2021

I'll nuke this one and create a fresh pull request once I'm confident the action does not pick up false-positives

@cart
Copy link
Member

cart commented May 6, 2021

Just fixed the git history by squashing the commits, rebasing on main, and force pushing.

@Weibye
Copy link
Contributor Author

Weibye commented May 28, 2021

Sorry for the delay everyone, this is almost complete. Will aim to have the pull request finished by the end of the day today.

@Weibye
Copy link
Contributor Author

Weibye commented May 28, 2021

I had the action working properly, then #1714 came along and I realized I needed to support examples that does not lie within the examples folder, which has been some of the reason for this delay

@Weibye
Copy link
Contributor Author

Weibye commented May 28, 2021

I think this is ready now and should be ready to go as #2263 gets merged.

@Weibye Weibye marked this pull request as ready for review May 28, 2021 19:44
@mockersf
Copy link
Member

bors try

so it should fail now right?

bors bot added a commit that referenced this pull request May 28, 2021
@mockersf
Copy link
Member

It works, it fails! https://github.com/bevyengine/bevy/runs/2697857713

Could you add it to bors? https://github.com/bevyengine/bevy/blob/main/.github/bors.toml

@MinerSebas
Copy link
Contributor

Shouldn't this also currently Fail on the Tests category, as it isn't stored in the examples Folder? (See the ../)

## Tests

Example | File | Description
--- | --- | ---
`how_to_test_systems` | [`../tests/how_to_test_systems.rs`](../tests/how_to_test_systems.rs) | How to test systems with commands, queries or resources

But the CI config only sets this: source: './examples/'

@Weibye
Copy link
Contributor Author

Weibye commented May 28, 2021

Shouldn't this also currently Fail on the Tests category, as it isn't stored in the examples Folder? (See the ../)

## Tests

Example | File | Description
--- | --- | ---
`how_to_test_systems` | [`../tests/how_to_test_systems.rs`](../tests/how_to_test_systems.rs) | How to test systems with commands, queries or resources

But the CI config only sets this: source: './examples/'

Hmm. Yes you are right. I'll fix this in the morning.

What is the desired approach here? Should we force test examples inside the examples folder? Or are test examples another thing that should be ignored?

@Weibye Weibye marked this pull request as draft May 29, 2021 09:38
@Weibye
Copy link
Contributor Author

Weibye commented May 29, 2021

@mockersf Are there any specific reason why the examples on how to test systems (#1714) should live in the Test folder instead of the Examples folder?

@Weibye
Copy link
Contributor Author

Weibye commented May 29, 2021

  • The action should now work and correctly fail on account of Test examples are not located within the scope of examples/
  • iter_combinations should no longer be picked up as iter_combinations example missing from README #2263 has been merged
  • I also improved the output messages a bit to hopefully add some clarity

Could you add it to bors? https://github.com/bevyengine/bevy/blob/main/.github/bors.toml

I've also added the action to bors

@bors
Copy link
Contributor

bors bot commented May 29, 2021

try

Build failed:

@Weibye
Copy link
Contributor Author

Weibye commented May 29, 2021

Build failed:
example-doc-validation

Should anyone have a suggestion for a better / clearer name, I'm all ears

@mockersf
Copy link
Member

@mockersf Are there any specific reason why the examples on how to test systems (#1714) should live in the Test folder instead of the Examples folder?

To be executed when running cargo test

@Weibye
Copy link
Contributor Author

Weibye commented May 30, 2021

@mockersf Are there any specific reason why the examples on how to test systems (#1714) should live in the Test folder instead of the Examples folder?

To be executed when running cargo test

That makes sense, in which case I propose #2281 in order to unblock this PR

@mockersf
Copy link
Member

mockersf commented May 30, 2021

I didn't check how your action works, but if it fails on that it may means it does things backwards: The goal of this PR is to check that examples are in the README.md, not that the examples listed in this file are present in the examples folder.
The fact that those links are valid should already be checked by our link checker in CI

@Weibye
Copy link
Contributor Author

Weibye commented May 30, 2021

I didn't check how your action works, but if it fails on that it may means it does things backwards: The goal of this PR is to check that examples are in the README.md, not that the examples listed in this file are present in the examples folder.

Currently it does both.

The fact that those links are valid should already be checked by our link checker in CI

The link-checker CI didn't exist at the time I started this work, so I reckoned checking for invalid links would be required to properly solve the problem. I'll remove that check from the action as it is redundant now.

This action will then only check that files within ./examples are present in ./examples/README.md and Cargo.toml, and not the other way around.

@Weibye Weibye marked this pull request as draft May 30, 2021 13:22
@bors
Copy link
Contributor

bors bot commented May 30, 2021

🔒 Permission denied

Existing reviewers: click here to make Weibye a reviewer

@Weibye Weibye marked this pull request as ready for review May 30, 2021 14:16
@Weibye
Copy link
Contributor Author

Weibye commented May 30, 2021

I think this should now be complete and work properly this time.

  • Renamed the action, hopefully to be more clear.
  • Re-versioned the action to get rid of the old non-working builds

This action should now run without picking up any issues and be ready to merge.

@NathanSWard
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request May 30, 2021
@bors
Copy link
Contributor

bors bot commented May 30, 2021

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 30, 2021
@cart
Copy link
Member

cart commented May 30, 2021

bors r+

@cart
Copy link
Member

cart commented May 30, 2021

Great work! I feel so safe now :)

bors bot pushed a commit that referenced this pull request May 30, 2021
Closes #1581 

# Internal File/Link Consistency checker Action

This pull request adds an action to the CI that parses the [`./examples`](https://github.com/bevyengine/bevy/tree/main/examples) folder for files and cross references that with the links listed in [`README.md`](https://github.com/bevyengine/bevy/blob/main/examples/README.md) and [`Cargo.toml`](https://github.com/bevyengine/bevy/blob/main/Cargo.toml) to ensure the documentation actually reflects the examples currently in the repo. 

The primary reason for why we want this, is to prevent people from adding new examples but forgetting to also list them in the docs, or accidentally entering broken links (typos in docs).

For details on how the action is working: [Check out the README here](https://github.com/Weibye/action-internal-link-consistency/blob/main/README.md)

Co-authored-by: Andreas Weibye <[email protected]>
@bors bors bot changed the title CI - Check that examples are listed in README and Cargo [Merged by Bors] - CI - Check that examples are listed in README and Cargo May 30, 2021
@bors bors bot closed this May 30, 2021
@Weibye Weibye deleted the ci_examples_improvements branch May 31, 2021 18:19
@DJMcNab
Copy link
Member

DJMcNab commented Jul 16, 2021

@Weibye please respond in #2373 for the relicense to MIT/Apache 2.0. Thanks!

@Weibye
Copy link
Contributor Author

Weibye commented Jul 16, 2021

@Weibye please respond in #2373 for the relicense to MIT/Apache 2.0. Thanks!

Oof, thought I already had. Thanks for the reminder!

@DJMcNab
Copy link
Member

DJMcNab commented Jul 16, 2021

Yay! Thanks! We missed you in the initial checks for users, but we're doing additional checks and found this PR from that.

ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
)

Closes bevyengine#1581 

# Internal File/Link Consistency checker Action

This pull request adds an action to the CI that parses the [`./examples`](https://github.com/bevyengine/bevy/tree/main/examples) folder for files and cross references that with the links listed in [`README.md`](https://github.com/bevyengine/bevy/blob/main/examples/README.md) and [`Cargo.toml`](https://github.com/bevyengine/bevy/blob/main/Cargo.toml) to ensure the documentation actually reflects the examples currently in the repo. 

The primary reason for why we want this, is to prevent people from adding new examples but forgetting to also list them in the docs, or accidentally entering broken links (typos in docs).

For details on how the action is working: [Check out the README here](https://github.com/Weibye/action-internal-link-consistency/blob/main/README.md)

Co-authored-by: Andreas Weibye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CI that checks whether new Examples are in readme
9 participants