Skip to content

Conversation

@RaoulLuque
Copy link
Member

This addresses the problem with CONTRIBUTING.md found in #844.

Additionally, further details about CI actions which might be interesting to contributors were added such as miri, as well as running tests on different toolchains / rust versions.

Resolves #844

@RaoulLuque RaoulLuque added A-readme Area: Documentation that isn't part of any crate such as README.md or CONTRIBUTING.rst S-waiting-on-review Status: Awaiting review from the assignee but also other interested parties labels Jul 6, 2025
@starovoid starovoid added this to the 0.8.3 milestone Jul 6, 2025
Copy link
Member

@ABorgna ABorgna left a comment

Choose a reason for hiding this comment

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

I'm wondering if we should add a justfile so calling just test / just check / just miri adds the appropriate flags.

@RaoulLuque
Copy link
Member Author

Ah, good point 🤔 I have to say that I am not the biggest fan of introducing non-cargo tooling, but since one can install it via cargo install, I'd be in favor as well :)

I'll rename this PR then ^^ I suppose I'd also change the CI and everything as well then, to use them everywhere, right?

@RaoulLuque RaoulLuque changed the title docs: Add lints and other tools used in CI to CONTRIBUTING.md ci: Use just and adapt CONTRIBUTING.md and ci.yml accordingly Jul 7, 2025
@RaoulLuque
Copy link
Member Author

Okay, should be adapted accordingly now :) Let me know if there's anything you think we should include / change 🦕

@RaoulLuque RaoulLuque requested a review from ABorgna July 7, 2025 14:31
@starovoid starovoid force-pushed the add_lints_to_contributing_md branch from e6ddd55 to d0b231b Compare July 13, 2025 15:50
Copy link
Collaborator

@starovoid starovoid left a comment

Choose a reason for hiding this comment

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

Everything is good and beautiful except just build command.

@RaoulLuque
Copy link
Member Author

Alright, thanks :) I felt like it could be nice to have one command which contributors can use which checks most of the things in one, so I added the just ci job, which just runs cargo test and the lints in one and adapted the CONTRIBUTING.md accordingly :)

Btw, I forgot to add your suggestion here in Github, so now it's in my commit instead, sry :o

@RaoulLuque RaoulLuque requested a review from starovoid July 26, 2025 14:38
github-merge-queue bot pushed a commit that referenced this pull request Jul 27, 2025
This PR fixes some clippy lints about lifetime annotations which occur
when running clippy like it is run in CI using the nightly toolchain.

More precisely, when running:
```bash
cargo clippy --all-features --lib --bins --examples --tests -- -D warnings
```
or alternatively `just clippy` when #845 is merged, one gets multiple
warnings (Errors because of `-D warnings`) of the following sort:
```bash
error: lifetime flowing from input to output with different syntax can be confusing
   --> src/adj.rs:314:25
    |
314 |     pub fn edge_indices(&self) -> EdgeIndices<E, Ix> {
    |                         ^^^^^     ------------------ the lifetime gets resolved as '_
    |                         |
    |                         this lifetime flows to the output
    |
    = note: -D mismatched-lifetime-syntaxes implied by -D warnings
    = help: to override -D warnings add #[allow(mismatched_lifetime_syntaxes)]
help: one option is to remove the lifetime for references and use the anonymous lifetime for paths
    |
314 |     pub fn edge_indices(&self) -> EdgeIndices<'_, E, Ix> {
    |                                               +++
```

This PR fixes these lints by exactly adding an anonymous lifetime to the
return types and thus making the lifetime flow more explicit.
Furthermore, a tiny change in the order of `std` and `no-std` generic
parameters in `matrix_graph.rs` is made to make RustRover lints happy.
@RaoulLuque
Copy link
Member Author

@starovoid do you accept the new version :) ? Because GitHub is still showing you as requesting changes and I don't want to just dismiss your request ^^

@starovoid
Copy link
Collaborator

@starovoid do you accept the new version :) ? Because GitHub is still showing you as requesting changes and I don't want to just dismiss your request ^^

Everything looks great, I just haven't had a chance to look at it yet ^^

@RaoulLuque
Copy link
Member Author

Ah, sry. I missunderstood then 😅

@RaoulLuque RaoulLuque force-pushed the add_lints_to_contributing_md branch from a35e630 to 6345278 Compare July 27, 2025 14:23
@RaoulLuque RaoulLuque force-pushed the add_lints_to_contributing_md branch from 6345278 to e77aabc Compare July 27, 2025 16:34
@starovoid starovoid added this pull request to the merge queue Jul 27, 2025
Merged via the queue into petgraph:master with commit f268b4f Jul 27, 2025
15 of 20 checks passed
RaoulLuque added a commit to RaoulLuque/petgraph that referenced this pull request Sep 21, 2025
…ph#845)

This addresses the problem with CONTRIBUTING.md found in petgraph#844. 

Additionally, further details about CI actions which might be
interesting to contributors were added such as miri, as well as running
tests on different toolchains / rust versions.

Resolves petgraph#844
RaoulLuque added a commit to RaoulLuque/petgraph that referenced this pull request Sep 21, 2025
This PR fixes some clippy lints about lifetime annotations which occur
when running clippy like it is run in CI using the nightly toolchain.

More precisely, when running:
```bash
cargo clippy --all-features --lib --bins --examples --tests -- -D warnings
```
or alternatively `just clippy` when petgraph#845 is merged, one gets multiple
warnings (Errors because of `-D warnings`) of the following sort:
```bash
error: lifetime flowing from input to output with different syntax can be confusing
   --> src/adj.rs:314:25
    |
314 |     pub fn edge_indices(&self) -> EdgeIndices<E, Ix> {
    |                         ^^^^^     ------------------ the lifetime gets resolved as '_
    |                         |
    |                         this lifetime flows to the output
    |
    = note: -D mismatched-lifetime-syntaxes implied by -D warnings
    = help: to override -D warnings add #[allow(mismatched_lifetime_syntaxes)]
help: one option is to remove the lifetime for references and use the anonymous lifetime for paths
    |
314 |     pub fn edge_indices(&self) -> EdgeIndices<'_, E, Ix> {
    |                                               +++
```

This PR fixes these lints by exactly adding an anonymous lifetime to the
return types and thus making the lifetime flow more explicit.
Furthermore, a tiny change in the order of `std` and `no-std` generic
parameters in `matrix_graph.rs` is made to make RustRover lints happy.
RaoulLuque added a commit to RaoulLuque/petgraph that referenced this pull request Sep 21, 2025
…ph#845)

This addresses the problem with CONTRIBUTING.md found in petgraph#844. 

Additionally, further details about CI actions which might be
interesting to contributors were added such as miri, as well as running
tests on different toolchains / rust versions.

Resolves petgraph#844
@RaoulLuque RaoulLuque deleted the add_lints_to_contributing_md branch September 22, 2025 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-readme Area: Documentation that isn't part of any crate such as README.md or CONTRIBUTING.rst S-waiting-on-review Status: Awaiting review from the assignee but also other interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better documentation of clippy lints

3 participants