Skip to content

Conversation

@mmagician
Copy link
Contributor

@mmagician mmagician commented Sep 1, 2025

Describe your changes

EDIT: I'm not 100% convinced about re-using the workflows. It makes the CI much less transparent (one needs to look outside of this repo to find out which jobs are actually run as part of linting).

Checklist before requesting a review

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.

@bobbinth bobbinth added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Sep 1, 2025
/// Returns an iterator over the [`InnerNode`] and the respective [`NodeIndex`] of the
/// [`PartialSmt`].
pub fn inner_node_indicies(&self) -> impl Iterator<Item = (NodeIndex, InnerNode)> + '_ {
pub fn inner_node_indices(&self) -> impl Iterator<Item = (NodeIndex, InnerNode)> + '_ {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

so this was added in #494 to support 0xMiden/miden-node#1158, but (for now?) the latter doesn't use these getters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we still need to propagate the dependencies to miden base/node.

@mmagician mmagician requested a review from bobbinth September 1, 2025 18:40
@mmagician mmagician changed the title chore: reuse shared lint workflows chore: expand & re-use lint workflows Sep 1, 2025
@mmagician mmagician changed the title chore: expand & re-use lint workflows chore: expand & re-use lint CI workflows Sep 1, 2025
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left one small comment inline and also seems to have broken the build when trying to resolve merge conflicts (sorry!). Once these are addressed, we should be good to merge.

/// Returns an iterator over the [`InnerNode`] and the respective [`NodeIndex`] of the
/// [`PartialSmt`].
pub fn inner_node_indicies(&self) -> impl Iterator<Item = (NodeIndex, InnerNode)> + '_ {
pub fn inner_node_indices(&self) -> impl Iterator<Item = (NodeIndex, InnerNode)> + '_ {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we still need to propagate the dependencies to miden base/node.

@mmagician mmagician force-pushed the mmagician-reuse-workflows branch from 0bdc589 to 7b29d71 Compare September 3, 2025 08:32
@mmagician mmagician merged commit 9019e8c into 0xMiden:next Sep 3, 2025
19 checks passed
@mmagician mmagician deleted the mmagician-reuse-workflows branch September 3, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants