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.

Sign up for GitHub

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

Jump to bottom

Advance chaintips changes #1506

Merged
merged 18 commits into from
Jul 26, 2024
Merged

Advance chaintips changes #1506

merged 18 commits into from
Jul 26, 2024

Conversation

tippenein
Copy link
Collaborator

updates advance_chaintip to handle burn and stacks chains separately

@tippenein tippenein marked this pull request as draft July 15, 2024 04:58
@tippenein tippenein force-pushed the advance-chaintips-changes branch 2 times, most recently from 73af6ed to 4cb96ec Compare July 15, 2024 19:58
@tippenein tippenein marked this pull request as ready for review July 16, 2024 18:51
@tippenein tippenein marked this pull request as draft July 16, 2024 19:18
@tippenein tippenein force-pushed the advance-chaintips-changes branch 3 times, most recently from 04274da to 3983559 Compare July 18, 2024 16:25
@tippenein tippenein marked this pull request as ready for review July 18, 2024 16:25
@tippenein tippenein marked this pull request as draft July 18, 2024 17:17
@tippenein tippenein force-pushed the advance-chaintips-changes branch 3 times, most recently from e47ad7c to 4374d79 Compare July 19, 2024 16:19
@codecov Codecov
Copy link

codecov bot commented Jul 19, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 93.93939% with 10 lines in your changes missing coverage. Please review.

Files Patch % Lines
components/clarity-repl/src/repl/session.rs 88.76% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

@tippenein tippenein marked this pull request as ready for review July 19, 2024 16:39
@hugocaillard hugocaillard self-requested a review July 19, 2024 16:47
hugocaillard
hugocaillard requested changes Jul 22, 2024
View reviewed changes
Copy link
Collaborator

@hugocaillard hugocaillard left a comment

Choose a reason for hiding this comment

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

A few change requests in the tests and couple of nitpicks.
Also, could you make sure that the new methods and session are covered by tests? https://app.codecov.io/gh/hirosystems/clarinet/pull/1506/blob/components/clarity-repl/src/repl/session.rs#L824 (parse_and_advance_stacks_chaintip & parse_and_advance_burn_chaintip

components/clarinet-sdk/node/tests/fixtures/Clarinet.toml Outdated Show resolved Hide resolved
components/clarity-repl/src/repl/datastore.rs Outdated Show resolved Hide resolved
components/clarinet-sdk/node/tests/simnet-usage-epoch3.test.ts Outdated Show resolved Hide resolved
components/clarinet-sdk/node/tests/simnet-usage-epoch3.test.ts Outdated Show resolved Hide resolved
components/clarity-repl/src/repl/interpreter.rs Outdated Show resolved Hide resolved
components/clarity-repl/src/repl/session.rs Outdated Show resolved Hide resolved
hugocaillard

This comment was marked as outdated.

Sign in to view

hugocaillard
hugocaillard requested changes Jul 23, 2024
View reviewed changes
Copy link
Collaborator

@hugocaillard hugocaillard left a comment

Choose a reason for hiding this comment

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

I don't know if we should enforce it, but it would be nice to have new lines add the end of each files

@hugocaillard hugocaillard self-requested a review July 23, 2024 16:39
@hugocaillard hugocaillard self-requested a review July 25, 2024 14:56
hugocaillard
hugocaillard approved these changes Jul 26, 2024
View reviewed changes
Copy link
Collaborator

@hugocaillard hugocaillard left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@hugocaillard hugocaillard merged commit efeadc5 into main Jul 26, 2024
19 checks passed
@hugocaillard hugocaillard deleted the advance-chaintips-changes branch July 26, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugocaillard hugocaillard hugocaillard approved these changes

Assignees
No one assigned
Labels
None yet
Projects
DevTools
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add advance_stacks_tip and advance_burnchain_tip in simnet
2 participants
@tippenein @hugocaillard