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

style: Increased append readability #1350

Merged
merged 5 commits into from
Jan 15, 2024
Merged

style: Increased append readability #1350

merged 5 commits into from
Jan 15, 2024

Conversation

deelawn
Copy link
Contributor

@deelawn deelawn commented Nov 9, 2023

I've been looking at append code a lot recently and the current variable naming makes it difficult for me to quickly understand what I'm looking at; these changes help.

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

Copy link

netlify bot commented Nov 9, 2023

Deploy Preview for gno-docs2 canceled.

Name Link
🔨 Latest commit f5a3c56
🔍 Latest deploy log https://app.netlify.com/sites/gno-docs2/deploys/654d36244c338e0008aa1e64

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Nov 9, 2023
@deelawn deelawn requested a review from a team November 9, 2023 19:45
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Attention: 108 lines in your changes are missing coverage. Please review.

Comparison is base (d184938) 55.85% compared to head (8021e99) 55.85%.

Files Patch % Lines
gnovm/pkg/gnolang/uverse.go 37.57% 105 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1350   +/-   ##
=======================================
  Coverage   55.85%   55.85%           
=======================================
  Files         431      431           
  Lines       65839    65841    +2     
=======================================
+ Hits        36773    36775    +2     
  Misses      26185    26185           
  Partials     2881     2881           
Flag Coverage Δ
go-1.21.x ∅ <ø> (∅)
misc ∅ <ø> (∅)
misc-_test.genstd ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@zivkovicmilos zivkovicmilos 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 for making the uverse code a bit more readable 🙏

@deelawn
Copy link
Contributor Author

deelawn commented Dec 7, 2023

Will wait to merge after #1305 because it will introduce conflicts and will be easier to make the changes on this branch.

@thehowl
Copy link
Member

thehowl commented Jan 12, 2024

@deelawn can you merge & resolve conflicts?

@deelawn
Copy link
Contributor Author

deelawn commented Jan 12, 2024

There were some issues while merging in master but they should all be fixed now. Anyone want to give this one more look before I merge it?

@thehowl thehowl merged commit e4bad9c into master Jan 15, 2024
188 of 189 checks passed
@thehowl thehowl deleted the style/append branch January 15, 2024 22:18
gfanton pushed a commit to moul/gno that referenced this pull request Jan 18, 2024
I've been looking at append code a lot recently and the current variable
naming makes it difficult for me to quickly understand what I'm looking
at; these changes help.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants