Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🚚 Rename burn_chain_tip to burnchain_tip #3598

Conversation

r0zar
Copy link
Contributor

@r0zar r0zar commented Mar 2, 2023

Incremental refactor PR to clean up deprecated variable name.

Description

Rename burn_chain_tip to burnchain_tip to normalize naming conventions

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

@CLAassistant
Copy link

CLAassistant commented Mar 2, 2023

CLA assistant check
All committers have signed the CLA.

@igorsyl igorsyl requested review from jio-gl and removed request for jio-gl March 2, 2023 15:25
@igorsyl
Copy link
Contributor

igorsyl commented Mar 2, 2023

Thanks for helping make our code base better!

@codecov
Copy link

codecov bot commented Mar 2, 2023

Codecov Report

Merging #3598 (7977860) into master (9f7a650) will decrease coverage by 1.00%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #3598      +/-   ##
==========================================
- Coverage   31.23%   30.24%   -1.00%     
==========================================
  Files         298      298              
  Lines      275069   275069              
==========================================
- Hits        85923    83196    -2727     
- Misses     189146   191873    +2727     
Impacted Files Coverage Δ
src/burnchains/burnchain.rs 73.79% <75.00%> (-0.17%) ⬇️
testnet/stacks-node/src/tests/neon_integrations.rs 63.39% <0.00%> (-23.98%) ⬇️
testnet/stacks-node/src/tests/epoch_205.rs 80.18% <0.00%> (-17.97%) ⬇️
...mon/src/deps_common/bitcoin/blockdata/constants.rs 36.76% <0.00%> (-8.83%) ⬇️
testnet/stacks-node/src/event_dispatcher.rs 78.42% <0.00%> (-5.22%) ⬇️
...ommon/src/deps_common/bitcoin/network/serialize.rs 40.00% <0.00%> (-5.00%) ⬇️
src/chainstate/stacks/db/contracts.rs 38.46% <0.00%> (-3.85%) ⬇️
src/chainstate/burn/db/mod.rs 37.03% <0.00%> (-3.71%) ⬇️
testnet/stacks-node/src/tests/mod.rs 51.64% <0.00%> (-3.70%) ⬇️
stacks-common/src/util/macros.rs 78.00% <0.00%> (-2.80%) ⬇️
... and 45 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@jferrant jferrant left a comment

Choose a reason for hiding this comment

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

Looks like a good start! I like how you have kept your commits small/single change for easy review :) Look forward to seeing further commits/updates to the pull request!

@igorsyl igorsyl requested review from jio-gl and jcnelson March 6, 2023 16:30
@igorsyl
Copy link
Contributor

igorsyl commented Mar 6, 2023

Copy link
Contributor

@jio-gl jio-gl left a comment

Choose a reason for hiding this comment

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

all tests passed, a variable was renamed.

@jcnelson
Copy link
Member

jcnelson commented Mar 7, 2023

This looks fine to me; will approve when it's no longer a draft.

@r0zar r0zar marked this pull request as ready for review March 8, 2023 12:16
@r0zar
Copy link
Contributor Author

r0zar commented Mar 8, 2023

Since we've already got some 👀 on this- I'd like to see this PR through the code review / merge process while it's still small and low-risk. This will provide a great learning opportunity for me as this happens to be my first PR on the stack-blockchain repository.

To normalize naming conventions
@r0zar r0zar force-pushed the maint/burnchain-naming-cleanup branch from 9ffdfde to 7977860 Compare March 8, 2023 12:20
@r0zar
Copy link
Contributor Author

r0zar commented Mar 8, 2023

Rebased and ready for approval review. ✔️

@jcnelson
Copy link
Member

One of us will need to merge this into a fix/ branch so that the build CI steps will pass.

@kantai kantai changed the base branch from master to pointblankdev-maint/burnchain-naming-cleanup March 21, 2023 15:37
@kantai kantai merged commit cb59652 into stacks-network:pointblankdev-maint/burnchain-naming-cleanup Mar 21, 2023
kantai added a commit that referenced this pull request Mar 22, 2023
…chain-naming-cleanup

Merge: PR #3598 burnchain naming to `develop`
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

9 participants