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

docs: improve README emojis and quality section #384

Merged
merged 1 commit into from
Oct 3, 2023
Merged

docs: improve README emojis and quality section #384

merged 1 commit into from
Oct 3, 2023

Conversation

jeremylgn
Copy link
Contributor

Self explanatory.

@jeremylgn jeremylgn requested review from ccamel and bdeneux October 2, 2023 08:34
@jeremylgn jeremylgn self-assigned this Oct 2, 2023
@jeremylgn jeremylgn mentioned this pull request Oct 2, 2023
Closed
2 tasks
@bot-anik
Copy link
Member

bot-anik commented Oct 2, 2023 •
edited
Loading

size-limit report 📦

Path Size
target/wasm32-unknown-unknown/release/okp4_objectarium.wasm 425.7 KB (0%)
target/wasm32-unknown-unknown/release/okp4_law_stone.wasm 634.32 KB (0%)
target/wasm32-unknown-unknown/release/okp4_cognitarium.wasm 791.91 KB (0%)

@codecov
Copy link

codecov bot commented Oct 2, 2023 •
edited
Loading

Codecov Report

Merging #384 (173a7a0) into main (a33fd65) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #384   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files          42       42           
  Lines       10081    10081           
=======================================
  Hits         9891     9891           
  Misses        190      190

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

bdeneux
bdeneux reviewed Oct 2, 2023
View reviewed changes
Copy link
Contributor

@bdeneux bdeneux left a comment

Choose a reason for hiding this comment

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

Just some suggestions open to discussion before approve or request changes :)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
ccamel
ccamel requested changes Oct 2, 2023
View reviewed changes
Copy link
Member

@ccamel ccamel left a comment

Choose a reason for hiding this comment

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

@jeremylgn please add in this PR the section to describe the quality assurance approach for smart contracts (as specified in #341).

@jeremylgn jeremylgn requested review from bdeneux and ccamel October 2, 2023 13:01
bdeneux
bdeneux approved these changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@bdeneux bdeneux left a comment

Choose a reason for hiding this comment

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

Beautiful 👨‍🎨

amimart
amimart approved these changes Oct 2, 2023
View reviewed changes
Copy link
Member

@amimart amimart left a comment

Choose a reason for hiding this comment

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

Nice!

ccamel
ccamel requested changes Oct 2, 2023
View reviewed changes
Copy link
Member

@ccamel ccamel left a comment

Choose a reason for hiding this comment

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

Just a minor comment, but it's up to you to decide.

Concerning the commits, both messages are nearly the same, leading to ambiguity in the commit history narrative. I recommend either dividing the two features (emojis and quality section) into separate commits with suitable changes and clear commit messages or squasing the two commits into a single one. Once again, it's up to you to decide. 😌

README.md Outdated Show resolved Hide resolved
@jeremylgn jeremylgn force-pushed the docs/improve-readme-emojis-quality-section branch from 34ac829 to 173a7a0 Compare October 2, 2023 14:33
@jeremylgn jeremylgn requested a review from ccamel October 2, 2023 14:34
ccamel
ccamel approved these changes Oct 2, 2023
View reviewed changes
Copy link
Member

@ccamel ccamel left a comment

Choose a reason for hiding this comment

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

All good! Thanks 🙏

@jeremylgn jeremylgn merged commit 56d141a into main Oct 3, 2023
@jeremylgn jeremylgn deleted the docs/improve-readme-emojis-quality-section branch October 3, 2023 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ErikssonJoakim ErikssonJoakim ErikssonJoakim approved these changes

@bdeneux bdeneux bdeneux approved these changes

@amimart amimart amimart approved these changes

@ccamel ccamel ccamel approved these changes

Assignees

@jeremylgn jeremylgn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jeremylgn @bot-anik @ccamel @amimart @bdeneux @ErikssonJoakim