Skip to content

Conversation

@volodymyrd
Copy link
Contributor

@volodymyrd volodymyrd commented Feb 4, 2025

Extends Config for rankdir support - direction of graph layout.

BREAKING CHANGE:

Add new variant of public enum dot::Config.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2025

This PR contains breaking changes to the public Rust API.

cargo-semver-checks summary

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/enum_variant_added.ron

Failed in:
variant Config:RankDir in /home/runner/work/petgraph/petgraph/PR_BRANCH/src/dot.rs:131

@ABorgna ABorgna added the S-breaking-change Status: Need a breaking change release to progress label Feb 5, 2025
@ABorgna ABorgna added this to the 0.8 milestone Feb 5, 2025
@starovoid starovoid mentioned this pull request Mar 21, 2025
8 tasks
@starovoid starovoid changed the title Add support for specifying rankdir on dot plots. feat!: Add support for specifying rankdir on dot plots. Mar 24, 2025
@starovoid
Copy link
Collaborator

Hi @volodymyrd, please rebase and resolve conflicts.

@volodymyrd
Copy link
Contributor Author

volodymyrd commented Mar 24, 2025

Hi @starovoid done

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.

Please fix the following warning from clippy:

error: this URL is not a hyperlink
   --> src/dot.rs:100:5
    |
100 | /// https://graphviz.org/docs/attrs/rankdir/
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: bare URLs are not automatically turned into clickable links
    = note: `-D rustdoc::bare-urls` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(rustdoc::bare_urls)]`
help: use an automatic link instead
    |
100 | /// <https://graphviz.org/docs/attrs/rankdir/>
    |     +                                        +

@volodymyrd
Copy link
Contributor Author

done
Interesting, I have run clippy locally (cargo clippy) and did not get any warnings

@starovoid
Copy link
Collaborator

Interesting, I have run clippy locally (cargo clippy) and did not get any warnings

Workflow uses more detailed checks:

- name: Run clippy
        # The benchmarks target require nightly,
        # so we cannot use --all-targets here.
        run: |
          cargo clippy --all-features \
            --lib --bins --examples --tests \
            -- -D warnings

- name: Build docs
        run: cargo doc --no-deps --all-features
        env:
          RUSTDOCFLAGS: "-Dwarnings"

@starovoid
Copy link
Collaborator

Finally, please squash all commits into new one to simplify commits history.
You can use the following commands:

git reset $(git merge-base master $(git branch --show-current))
git add -A
git commit -m "Add support for specifying rankdir on dot plots"
git push --force

@starovoid starovoid self-requested a review March 24, 2025 16:13
Comment on lines 132 to 133
#[doc(hidden)]
_Incomplete(()),
Copy link
Member

Choose a reason for hiding this comment

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

Since we're doing a breaking change adding this already, can you add #[non_exhaustive] to the enum?

And remove this workaround:

Suggested change
#[doc(hidden)]
_Incomplete(()),

@ABorgna
Copy link
Member

ABorgna commented Mar 24, 2025

Finally, please squash all commits into new one to simplify commits history.

No need for that, the merge queue will automatically squash and rebase :)

@volodymyrd
Copy link
Contributor Author

Should I merge the changes?

@ABorgna
Copy link
Member

ABorgna commented Mar 24, 2025

Let's merge it, I'll add the enum flag in a separate PR
Thanks!

@ABorgna ABorgna added this pull request to the merge queue Mar 24, 2025
Merged via the queue into petgraph:master with commit 39ea4d9 Mar 24, 2025
10 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Mar 24, 2025
Removes the TODO as mentioned in #728

BREAKING CHANGE: Marked `dot::Enum` as `non_exhaustive`
github-merge-queue bot pushed a commit that referenced this pull request Apr 5, 2025
A sufficient number of proposed changes have accumulated to combine them
and publish a new major release numbered `0.8.0`.


BREAKING CHANGE:

This will require the user to provide extra type parameter in some APIs
(Read more in #747).

## List of changes
- [x] #747
The main innovation of the current release, the long-awaited feature
that has become very relevant due to the transition of dependent
projects to support `no_std`.
- [x] #662 
- [x] #611
- [x] #728
- [x] #686
- [x] #737 
- [x] #720 
- [x] #718 

## Note
There are still a large number of PRs that we want to adopt in the near
future, so we should expect at least a release of `0.8.1` soon after the
completion of `0.8.0`.

Thank you all for participating!

---------

Co-authored-by: Agustin Borgna <agustinborgna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-breaking-change Status: Need a breaking change release to progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants