JuliaDocs / DemoCards.jl

Let's focus on writing demos
MIT License
66 stars 14 forks source link

v0.5.3 is a breaking change for automatically generated covers #156

Closed brucala closed 12 months ago

brucala commented 12 months ago

I noticed that v0.53 was a breaking change for Deneb.jl in the sense that all demo card covers appeared with the fallback image.

After some digging I found #131. If I understand correctly, this was an intended breaking change and all packages that automatically generate covers must add the tag generate_cover: true to the cards (that automatically generate covers). Just wanted to ask if this was really the intended behavior? If it was, could the docs please be updated :pray:? and maybe also add more visibility about this breaking change in the release changelogs? That would greatly help developers using DemoCards figure out why their cards don't have covers anymore. I suspect most of the affected packages didn't notice it yet because the last built the docs months ago. Hopefully, having this issue open will also add a bit more visibility about it.

(BTW, thanks for the great DemoCards.jl)

frankier commented 12 months ago

Hi! I will take a deeper look at this soon, but just to note already that -- despite it being quite poorly communicated -- I changed that PR to attempt to be non breaking -- to generate the cover whenever an image isn't given since on balance that is the most common usage of this package. This is why I only bumped the patch version. I probably should have bumped a minor version rather than just patch anyway since there was the potential for breakage. Hopefully it can be patched again to not be breaking.

frankier commented 12 months ago

Sorry about this. I decided on balance to try and not make a breaking change, but tried to get clever with it at the last minute and go for some in-between approach which it turns out doesn't make a whole lot of sense.

I have set this new option to false now here https://github.com/JuliaDocs/DemoCards.jl/commit/773a519348444de9ad1573801f3207870f136a38

Can you please revert your most recent change and test that commit with Deneb.jl to double check it is okay? If it addresses everything I will release it as v0.5.4. Hopefully most people will skip v0.5.3. This is probably less painful than trying to yank v0.5.3.

brucala commented 12 months ago

Hi @frankier, no need to apologize, this is a 0. version and we, opportunistic users :sweat_smile:, should expect things like this to happen from time to time. Thanks for your contributions to DemoCards and help make it even better, and for taking these issues seriously and making an effort to give us as smooth an experience as possible :pray:.

I think your solution should work great, I'll try to find sometime tonight to try it out and let you know.

brucala commented 12 months ago

Hi @frankier. Just tested 773a519 with my most recent change reverted and things worked as expected: I have autogenerated covers back. It would be great if you release it as v0.5.4, probably most people updating their docs to Documenter v1 will go directly to v0.5.4 and won't notice any problem.

Just another thought. Maybe the braking change actually makes sense and you want to move back to your previous solution (I don't have a strong opinion). In that case I would suggest to implement a way to change the default globally, so instead of adding generate_cover tags to all examples one could for instance just add a generate_cover=true to the makedemos call. Also, a mention to the generate_cover tag in this section would be welcome!

frankier commented 12 months ago

Should be fixed in v0.5.4. You're right that there are arguments both ways. Perhaps the current behaviour won't be a problem one day if we are able to not have to execute each script so many times over. At the moment though, this package is in maintenance mode.