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

chore: make unused block hashes in testing header creation explicit #5857

Merged
merged 5 commits into from
Feb 20, 2024

Conversation

tungleanh0902
Copy link
Contributor

@tungleanh0902 tungleanh0902 commented Feb 19, 2024

Description

closes: #3960


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against the correct branch (see CONTRIBUTING.md).
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/).
  • Added relevant godoc comments.
  • Provide a commit message to be used for the changelog entry in the PR description for review.
  • Re-reviewed Files changed in the Github PR explorer.
  • Review Codecov Report in the comment section below once CI passes.

@tungleanh0902
Copy link
Contributor Author

@nghuyenthevinh2000

@tungleanh0902 tungleanh0902 changed the title Make unused block hashes in testing header creation explicit Chore: Make unused block hashes in testing header creation explicit Feb 19, 2024
@tungleanh0902
Copy link
Contributor Author

@colin-axner

@tungleanh0902 tungleanh0902 changed the title Chore: Make unused block hashes in testing header creation explicit chore: Make unused block hashes in testing header creation explicit Feb 19, 2024
@tungleanh0902 tungleanh0902 changed the title chore: Make unused block hashes in testing header creation explicit chore: make unused block hashes in testing header creation explicit Feb 19, 2024
testing/values.go Outdated Show resolved Hide resolved
@nghuyenthevinh2000
Copy link
Contributor

nghuyenthevinh2000 commented Feb 19, 2024

Hi @crodriguezvega @colin-axner,

I hope that the IBC team will support some of our engineer contributions to ibc-go by approving workflows running. @tungle-notional is new to Cosmos, and he wants to put his engineering skill to test by learning to contribute to IBC - go. I will ensure that his contributions conform to conventional commits guidelines.

If this issue is irrelevent, we will look at other ones.

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1c0c756) 81.59% compared to head (7d44576) 81.59%.
Report is 35 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             main    #5857     +/-   ##
=========================================
  Coverage   81.59%   81.59%             
=========================================
  Files         199      198      -1     
  Lines       15167    12103   -3064     
=========================================
- Hits        12375     9876   -2499     
+ Misses       2326     1765    -561     
+ Partials      466      462      -4     

see 172 files with indirect coverage changes

Copy link
Contributor

@crodriguezvega crodriguezvega left a comment

Choose a reason for hiding this comment

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

Thank you, @tungle-notional! Welcome to Cosmos and looking forward to more contributions from you!

@DimitrisJim
Copy link
Contributor

Thank you @nghuyenthevinh2000 and welcome @tungle-notional!

One point of feedback I'd give back is to always make sure you first request to work on a feature by dropping a comment in the issue stating so. This is the easiest way to make sure work is not duplicated and does not go in vain and helps smooth out the contribution process. We have a contributing document https://github.com/cosmos/ibc-go/blob/main/CONTRIBUTING.md to get everyone up to speed.

Thanks again for opening the PR and welcome aboard! ❤️


UnusedHash = tmhash.Sum([]byte{0x00})
// unusedHash is a placeholder hash used for testing.
unusedHash = tmhash.Sum([]byte{0x00})
Copy link
Contributor

Choose a reason for hiding this comment

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

made this private! its used inside the testing package so we needn't expose it for the time being!

Copy link
Contributor

@DimitrisJim DimitrisJim left a comment

Choose a reason for hiding this comment

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

lgtm 🚀 thanks again!

@crodriguezvega crodriguezvega merged commit 72e2b6b into cosmos:main Feb 20, 2024
66 checks passed
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.

Make unused block hashes in testing header creation explicit
6 participants