Visitar URL original
Renamed `selectEnvTargets` to `selectGovTargets` and used it in place of `selectGovState` where possible by dancewithheart · Pull Request #5372 · IntersectMBO/ouroboros-network · GitHub
Skip to content

Renamed selectEnvTargets to selectGovTargets and used it in place of selectGovState where possible - #5372

Merged
coot merged 1 commit into
IntersectMBO:mainfrom
dancewithheart:drop-selectEnvTargets
May 20, 2026
Merged

coot merged 1 commit into
IntersectMBO:mainfrom
dancewithheart:drop-selectEnvTargets

Conversation

@dancewithheart

@dancewithheart dancewithheart commented May 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #4713

selectEnvTargets was already replaced by selectGovState in 3d270bd97

Renamed selectEnvTargets to selectGovTargets and used it in place of selectGovState where possible as @coot proposed in comment below.

Verified using:

cabal build all
cabal run cardano-diffusion:test:cardano-diffusion-sim-tests -- -p '/PeerSelection/'

Checklist

Quality

  • Commit sequence makes sense and have useful messages, see ref.
  • New tests are added and existing tests are updated.
  • Self-reviewed the PR.

Maintenance

  • Linked an issue or added the PR to the current sprint of ouroboros-network project.
  • Added labels.
  • Updated changelog files.
  • The documentation has been properly updated, see ref.

@dancewithheart
dancewithheart requested a review from a team as a code owner May 16, 2026 20:02
@github-project-automation github-project-automation Bot moved this to In Progress in Ouroboros Network May 16, 2026
@dancewithheart dancewithheart changed the title remove unused selectEnvTargets for #4713 Remove unused selectEnvTargets for #4713 May 16, 2026
@dancewithheart dancewithheart changed the title Remove unused selectEnvTargets for #4713 Remove unused selectEnvTargets May 16, 2026
@dancewithheart
dancewithheart force-pushed the drop-selectEnvTargets branch from 6c814c7 to db0f2ca Compare May 17, 2026 00:33
@coot coot added the outbound-governor Issues / PRs related to outbound-governor label May 18, 2026
@coot

coot commented May 18, 2026 •

Copy link
Copy Markdown
Contributor

I'd prefer to go in the other direction, e.g. use the selectEnvTargets where we can - it will make it easier for us to write new tests in the future.

The function should be called selectGovTargets as it reads the governor rather than the peer selection runtime environment.

@dancewithheart
dancewithheart marked this pull request as draft May 18, 2026 10:40
@dancewithheart
dancewithheart force-pushed the drop-selectEnvTargets branch from db0f2ca to 8b5eac6 Compare May 18, 2026 11:31
@dancewithheart dancewithheart changed the title Remove unused selectEnvTargets Renamed selectEnvTargets to selectGovTargets and used it in place of selectGovState where possible May 18, 2026
@dancewithheart
dancewithheart marked this pull request as ready for review May 18, 2026 11:41

@coot coot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@coot

coot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Could you:

git commit --amend --no-edit
git push --force-with-lease --force-if-includes

For some reason a notification from hydra to GitHub didn't arrive after hydra finished.

@dancewithheart
dancewithheart force-pushed the drop-selectEnvTargets branch from 8b5eac6 to 24a6fac Compare May 19, 2026 10:54
@dancewithheart

Copy link
Copy Markdown
Contributor Author

Synced with recent changes and rebased my branch.

I’ll watch CI, though I’m not fully sure which Hydra signal indicates that the completed build status was successfully reported back to GitHub.

@coot
coot added this pull request to the merge queue May 20, 2026
Merged via the queue into IntersectMBO:main with commit 54f75ba May 20, 2026
11 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Ouroboros Network May 20, 2026
@dancewithheart

Copy link
Copy Markdown
Contributor Author

Thank you for the review and merge 🙇
You made my day 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

outbound-governor Issues / PRs related to outbound-governor

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Use selectGovState instead of selectEnvTargets in PeerSelection tests

2 participants