Skip to content

Conversation

KolbyML
Copy link
Member

@KolbyML KolbyML commented Sep 5, 2025

With NIT-3732 OffchainLabs/nitro#3582

nitro-testnode only has to run --node.staker.strategy, not both --node.staker.strategy`` and --node.bold.strategy``

This PR in OffchainLabs/nitro-testnode will make CI pass for OffchainLabs/nitro#3582 docker CI

@Tristan-Wilson
Copy link
Member

The change itself looks fine. Have you read the guide about which branch to use?
https://github.com/OffchainLabs/nitro-testnode?tab=readme-ov-file#branch-selection-guide-for-devs-working-on-nitro-testnode

I think since this depends on an unreleased nitro version it should be on the master branch, but let me know what you think.

Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Marked for rework because maybe we need to change branch.

@KolbyML KolbyML changed the base branch from release to master September 9, 2025 14:53
@KolbyML
Copy link
Member Author

KolbyML commented Sep 9, 2025

@Tristan-Wilson I think I resolved the concern, ready for another look

@KolbyML KolbyML assigned Tristan-Wilson and unassigned KolbyML Sep 9, 2025
@Tristan-Wilson
Copy link
Member

Right now by changing the base branch in github it is comparing your branch against master, which includes a bunch of stuff from release that isn't in master. It's not to say that we shouldn't merge that stuff at some point, but we don't want to pick it up when we merge this PR which is a fairly targeted change.

I find having a git alias for drawing a graph of the revision history is super helpful for visualizing this. If you add this to your ~/.gitconfig

[alias]
        dag = log --graph --format='format:%C(yellow)%h%C(reset) %C(blue)\"%an\" <%ae>%C(reset) %C(magenta)%cr%C(reset)%C(auto)%d%C(reset)%n%s' --date-order

Then if you run git dag origin/master origin/upstream-changes you get some nice output like this where you can see exactly what's going on.

* 4a579e1 "Kolby Moroz Liebl" <[email protected]> 2 days ago (origin/upstream-changes)
| Remove node.bold.strategy use node.staker.strategy instead
*   34f0e1c "Pepper Lebeck-Jobe" <[email protected]> 7 days ago (origin/release, origin/HEAD)
|\  Merge pull request #145 from OffchainLabs/ha/traffic-flag
| * e148d9a "Henry" <[email protected]> 7 days ago (origin/ha/traffic-flag)
| | syntax fix
| * b7ccba4 "Henry" <[email protected]> 7 days ago
|/  add flags for l2 and l3 traffic
*   c7a5862 "Joshua Colvin" <[email protected]> 5 weeks ago
|\  Merge pull request #144 from OffchainLabs/ha/bump-token-bridge-sdk-dep
| * 6a93346 "Henry" <[email protected]> 5 weeks ago (origin/ha/bump-token-bridge-sdk-dep)
| | set default token bridge to 1.2.5
| * 3ba7308 "Henry" <[email protected]> 5 weeks ago
| | use ha/fix-missing-network-props
| * fed1d70 "TucksonDev" <[email protected]> 5 weeks ago
| | Update token-bridge-contracts tag
| * dc28df9 "Henry" <[email protected]> 6 weeks ago
| | fix token transfer script
| * 39869a1 "Henry" <[email protected]> 6 weeks ago
| | update default token bridge version
| * cab5714 "Henry" <[email protected]> 6 weeks ago
|/  fix null string weth address caused by token bridge sdk dep bump
*   aaa556d "Tristan-Wilson" <[email protected]> 7 weeks ago (origin/master)
|\  Merge pull request #143 from OffchainLabs/release-master-sync

You'll need to rebase your upstream-changes branch onto the current master and force push it.

$ git rebase --onto origin/master upstream-changes~ upstream-changes
$ git push --force origin upstream-changes

Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Needs rebasing onto master

@KolbyML KolbyML changed the base branch from master to release September 10, 2025 14:51
@KolbyML KolbyML changed the base branch from release to master September 10, 2025 14:58
@KolbyML KolbyML assigned Tristan-Wilson and unassigned KolbyML Sep 10, 2025
@KolbyML
Copy link
Member Author

KolbyML commented Sep 10, 2025

@Tristan-Wilson ready for another look

Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Nice

@Tristan-Wilson Tristan-Wilson merged commit f293759 into master Sep 11, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants