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

[1/n] - Logger trait v2. #505

Merged
merged 1 commit into from
Jun 17, 2021
Merged

[1/n] - Logger trait v2. #505

merged 1 commit into from
Jun 17, 2021

Conversation

Dirbaio
Copy link
Contributor

@Dirbaio Dirbaio commented Jun 11, 2021

Part 1 of N of #492

  • Panics on reentrant acquire.
  • Remove Write trait.

@Dirbaio Dirbaio force-pushed the logger-v2 branch 2 times, most recently from cae40a6 to 120abfd Compare June 11, 2021 13:27
@Dirbaio
Logger trait v2.
6cfd947 
- Panics on reentrant acquire.
- Remove `Write` trait.
@Dirbaio Dirbaio marked this pull request as ready for review June 11, 2021 13:39
@Dirbaio Dirbaio mentioned this pull request Jun 11, 2021
Closed
@Dirbaio
Copy link
Contributor Author

Dirbaio commented Jun 11, 2021

Code size comparison of the log snapshot test:

                               before    after    change
- debug:                       114320   107232     -6.2%
- release:                      43264    40852     -5.5%
- release + flags:              24288    21936     -9.6%
- release + flags + buildstd:   22860    20524    -10.2%

"flags" means these in Cargo.toml:

codegen-units = 1
debug = 2
debug-assertions = false
incremental = false
lto = 'fat'
opt-level = 'z'
overflow-checks = false

"buildstd" means these in .cargo/config.toml:

[unstable]
build-std = ["core"]
build-std-features = ["panic_immediate_abort"]

@Dirbaio Dirbaio mentioned this pull request Jun 13, 2021
Merged
@Dirbaio Dirbaio changed the title Logger trait v2. [1/n] - Logger trait v2. Jun 13, 2021
@Dirbaio Dirbaio mentioned this pull request Jun 14, 2021
Merged
@Urhengulas
Copy link
Member

@Dirbaio: Even though the PR is approved now, we will wait with merging and do one other patch release with some minor changes. After this is released we can start merging breaking changes as much as we want 😅😆

@Urhengulas Urhengulas added status: blocked Blocked on another issue, or on upstream libraries breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version labels Jun 16, 2021
@Dirbaio
Copy link
Contributor Author

Dirbaio commented Jun 16, 2021 •
edited
Loading

Sounds fair! Please don't let it grow many conflicts as rebasing these is super painful. 😅

@Urhengulas
Copy link
Member

Sounds fair! Please don't let it grow many conflicts as rebasing these is super painful. sweat_smile

We'll try to be quick! 😁

@Urhengulas
Copy link
Member

@Dirbaio: defmt 0.2.3 is released and therefore we are ready to merge breaking changes! 🎉

I will leave this to you or @jonas-schievink 😄

@Urhengulas Urhengulas removed the status: blocked Blocked on another issue, or on upstream libraries label Jun 17, 2021
@jonas-schievink
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 17, 2021

Build succeeded:

@bors bors bot merged commit e506711 into knurling-rs:main Jun 17, 2021
bors bot added a commit that referenced this pull request Jun 17, 2021
@bors @Dirbaio
Merge #507
ecf25b4 
507: [2/n] - Remove code-size-costly optimizations r=jonas-schievink a=Dirbaio

Part 2 of N of #492. Depends on #505 

- Remove bool compression
- Remove LEB128 compression. usize/isize are now 4 bytes, format tags are now 2 bytes.

Code size comparsion (only including gains from this PR, not #505):

```
                             before    after    change
debug:                       107232   101452    -5.3% 
release:                      40852    37396    -8.4%
release + flags:              21936    19416   -11.4%
release + flags + buildstd:   20524    18004   -12.3%

rustc 1.54.0-nightly (676ee1472 2021-05-06)
"flags" means these in Cargo.toml:
    codegen-units = 1
    debug = 2
    debug-assertions = false
    incremental = false
    lto = 'fat'
    opt-level = 'z'
    overflow-checks = false

"buildstd" means these in .cargo/config.toml:
    [unstable]
    build-std = ["core"]
    build-std-features = ["panic_immediate_abort"]
```

Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>
bors bot added a commit that referenced this pull request Jun 30, 2021
@bors @Dirbaio
Merge #508
5abfc33 
508: [5/n] Format trait v2 r=japaric a=Dirbaio

Part of #492 . Depends on ~~#505 #507 #521~~ #524

- Implemented new Format trait according to #492.
- Adjusted everything else accordingly.

Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>
@japaric japaric mentioned this pull request Jul 7, 2021
Closed
@Urhengulas Urhengulas mentioned this pull request Jul 21, 2021
Merged
7 tasks
bors bot added a commit that referenced this pull request Oct 19, 2021
@bors @Urhengulas
Merge #547
da1682f 
547: Migration guide `v0.2.x` to `v0.3.0` r=japaric a=Urhengulas

Migration guide from `defmt v0.2.x` to version `v0.3.0`.

https://deploy-preview-547--admiring-dubinsky-56dff5.netlify.app/migration-02-03.html

Fixes #530.

### TODO
- [x] #505: Logger trait v2
- [x] #521: [3/n] Remove u24
- [x] #522: Replace `µs` hint with `us`
- [x] (no adaption needed) ~~#508: [5/n] Format trait v2~~
- [x] #519: `DEFMT_LOG`
- [x] #550: `defmt::flush()`
- [x] knurling-rs/probe-run#198, knurling-rs/probe-run#250, 

Co-authored-by: Johann Hemmann <johann.hemmann@code.berlin>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonas-schievink jonas-schievink jonas-schievink approved these changes

Assignees

@jonas-schievink jonas-schievink

Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Dirbaio @Urhengulas @jonas-schievink