symmetryinvestments / imap-d

D library for IMAP (JMAP is a work-in-progress but the basics work)
14 stars 10 forks source link

JMAP for Dlang; get rid of Asdf and SIL dependency #110

Closed 9il closed 2 years ago

9il commented 2 years ago

I have added a workaround for the Windows bug. We just compile that code on with SIL or for Posix. That is bad, but anyway much better than just compile it only for SIL. That bug isn't related to Mir and these MRs.

kinke commented 2 years ago

I have added a workaround for the Windows bug.

Which one? https://github.com/symmetryinvestments/imap-d/issues/96?

9il commented 2 years ago

Which one? #96?

No. It is a very weird bug. Tests are failing if we add some code to compile (not tests). The same tests pass if we don't add another code. That is the requests code. I can't minimize it on ARM.

skoppe commented 2 years ago

I see more changes than just getting rid of asdf. This MR also removes a bunch of public xyzRaw functions, as well as change SilStruct to StringMap. All reasonable, but easier to review is separate.

I also wonder why this compiles given that it doesn't complete get rid of asdf.

codecov[bot] commented 2 years ago

Codecov Report

Merging #110 (1e71ebe) into master (0884bb5) will decrease coverage by 1.82%. The diff coverage is 0.00%.

:exclamation: Current head 1e71ebe differs from pull request most recent head f8d9c13. Consider uploading reports for the commit f8d9c13 to get more accurate results

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage   17.11%   15.29%   -1.83%     
==========================================
  Files          11       12       +1     
  Lines        1268     1419     +151     
==========================================
  Hits          217      217              
- Misses       1051     1202     +151     
Impacted Files Coverage Δ
source/jmap/api.d 0.00% <ø> (ø)
source/jmap/types.d 2.07% <0.00%> (-5.34%) :arrow_down:

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

kinke commented 2 years ago

LGTM from what I can tell (not familiar with asdf/mir-ion deser details). I've added a little Windows workaround required for SIL plugins against SIL >= v2.47, enabled all JMAP stuff on Windows too and renamed one changesRaw leftover to changes. SIL plugin CI is green too.

kinke commented 2 years ago

Okay I'll merge (needed for SIL). Most of the JMAP stuff seems to have been SIL-specific, so the breaking API changes probably don't really affect anyone else. And that can still be finalized/revised for the next tag.