search

JuliaGraphics / ColorTypes.jl

Basic color definitions and traits
Other
77 stars 35 forks source link

Remove redundant pre-register StyledStrings compat #307

Closed tecosaur closed 2 months ago

tecosaur commented 2 months ago

Now that StyledStrings is registered, we can do away with this code (and also include StyledStrings support in a release, which would be nice).

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.60%. Comparing base (142353d) to head (3a8f027). Report is 9 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #307 +/- ## ========================================== + Coverage 84.31% 85.60% +1.28% ========================================== Files 8 9 +1 Lines 778 806 +28 ========================================== + Hits 656 690 +34 + Misses 122 116 -6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kimikage commented 2 months ago

Note that there is also "test/Project.toml".

tecosaur commented 2 months ago

Hmmm, looks like I might need to make a patch release to the compat version of StyledStrings to improve the behaviour when precompiled within another package.

tecosaur commented 2 months ago

Yup, I'll do a 1.0.1 release shortly: see https://github.com/JuliaLang/StyledStrings.jl/commit/dc94cfaf159f5e5018c21be28242df31ecd846f7

kimikage commented 2 months ago

I will merge PR #308 first to make it easier to see the CI results. (Edit: Merged) (The problem on ARM is expected to be resolved with FixedPointNumbers v0.8.5.) Also, I believe Dependabot will be creating a PR in a couple of days, but it is not a priority.

tecosaur commented 2 months ago

Cool, I'd thought I'd have 1.0.1 out by now, but there seems to be a holdup to do with the casing of juialang in the repo. I'll update this PR once that's resolved.

kimikage commented 2 months ago

This is off-topic and outside my scope of responsibility, but what is the review process and accountability of StyledStrings.jl? Committing without PR deprives us of the opportunity to review. Of course, GitHub allows people to comment on commits, but that goes unnoticed by most people.

tecosaur commented 2 months ago

The way StyledStrings.jl is setup: Commits to Main go through PRs, always, even for small fixup commits that really don't need it. Commits to other branches (like julia1-compat) can just be pushed.

Currently, I'm hoping that other people will get involved with this, but it's pretty much a just-me show ATM. Given that, the review process is "I think it looks good".

tecosaur commented 2 months ago

The failing tests are from:

kimikage commented 2 months ago

https://github.com/JuliaGraphics/ColorTypes.jl/blob/e1954578012621c02f9607181474bc48121661d2/test/runtests.jl#L24-L29 Could we remove the try block?

tecosaur commented 2 months ago

I don't see why not, or why it was added in the first place?

kimikage commented 2 months ago

why it was added in the first place?

Because, as you know, julia1-compat didn't work before.