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

feat(encode/pattern): add debug and release formatters #301

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

SteveLauC
Copy link
Contributor

Fixes #300


What this PR does:

  1. Add D/debug and R/release pattern formatters.

@bconn98
Copy link
Collaborator

bconn98 commented Dec 2, 2023

@estk Recommended for merge

@SteveLauC
Copy link
Contributor Author

Thanks for having a look at this PR:)

@estk
Copy link
Owner

estk commented Dec 3, 2023

@SteveLauC excellent work, @bconn98 thanks for reviewing this.

@SteveLauC can you write some super simple tests just verifying in debug and release mode things work as expected?

@SteveLauC
Copy link
Contributor Author

Test added, commits squashed

@SteveLauC
Copy link
Contributor Author

SteveLauC commented Dec 5, 2023

Code formatted

Gentle ping on @bconn98 and @estk, PTAL

@bconn98
Copy link
Collaborator

bconn98 commented Dec 5, 2023

Quick check from mobile and kicking off the tests on my build server look good 👍🏻

@estk
Copy link
Owner

estk commented Dec 5, 2023

Lgtm with one exception, we need to build and run this test in CI. Maybe add another line to the Test action that does a 'cargo test --release --features simple_writer yourtestfile -- yourtestname'

.github/workflows/main.yml Outdated Show resolved Hide resolved
@SteveLauC
Copy link
Contributor Author

SteveLauC commented Dec 6, 2023

I recommend enabling CI even for first-time-contributors so that we can check if it works as expected

@bconn98
Copy link
Collaborator

bconn98 commented Dec 6, 2023

@SteveLauC Branch is out of date

estk
estk previously approved these changes Dec 6, 2023
Copy link
Owner

@estk estk left a comment

Choose a reason for hiding this comment

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

just needs conflicts resolved then gtg

@codecov-commenter
Copy link

Codecov Report

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

Comparison is base (b0a60a1) 61.17% compared to head (eb79f7a) 61.23%.

Files Patch % Lines
src/encode/pattern/mod.rs 69.56% 7 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
+ Coverage   61.17%   61.23%   +0.06%     
==========================================
  Files          23       23              
  Lines        1383     1406      +23     
==========================================
+ Hits          846      861      +15     
- Misses        537      545       +8     

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

@SteveLauC
Copy link
Contributor Author

Rebased

@estk estk merged commit 97c1347 into estk:main Dec 7, 2023
11 checks passed
@SteveLauC SteveLauC deleted the debug-release branch December 7, 2023 00:35
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.

[Feature Request]: debug and release pattern formatters
4 participants