Skip to content

Conversation

@Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Oct 21, 2025

Provided by Claude Sonnet 4.5 after longer discussions.

What I like:

  • FilteredPortFileProvider (damn was unnecessary)
  • refactor

What I don like:

  • Could be a bit less verbose.
  • probably I could fine tune it more.
  • renaming stuff was a bit slow

alternative to #1822

std::string msg;

// First, print ports that were excluded early (before dependency resolution)
for (const auto& excluded_spec : ports_to_exclude)
Copy link
Member

Choose a reason for hiding this comment

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

This is a good idea I missed in my version of the same #1822 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

In #1822 (comment) in addition to fixing this problem I also fixed the "Summary" at the end to properly note that these were EXCLUDED

@Neumann-A Neumann-A marked this pull request as ready for review October 21, 2025 22:20
@Neumann-A
Copy link
Contributor Author

should this be closed in favor of #1822 ?

@BillyONeal
Copy link
Member

should this be closed in favor of #1822 ?

Probably not, since I used ideas in here and am still looking at it a bit. Hope to have that ready for review with more tests and the edge cases the team asked me to investigate tomorrow.

@BillyONeal
Copy link
Member

Closing this as its effect was merged into #1822

@BillyONeal BillyONeal closed this Nov 12, 2025
@Neumann-A Neumann-A deleted the fix_features_of_skipped_deps_getting_added_to_ci branch November 13, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants