djpohly / dwl

dwm for Wayland - ARCHIVE: development has moved to Codeberg
https://codeberg.org/dwl/dwl
Other
1.93k stars 284 forks source link

Add GitHub action to update wiki depending on whether or not patches applicable to main #506

Closed wyatt-avilla closed 9 months ago

wyatt-avilla commented 10 months ago

Description

This PR adds a GitHub action that tests whether the patches within the wiki can be applied to main and adds an emoji next to their links accordingly. The script itself relies on each page within the wiki initially being formatted with sed -i '/https:\/\/github\.com\/[^/]\+\/[^/]\+\/compare\/[^/]\+\.patch/s/$/ [❔]/', this adds "[❔]" next to each link matching the expression. Each time the action is triggered, the script will download each patch and update the emoji within the square brackets according to whether or not git apply --check succeeds. For this to work with future patches, the patch template in Patches.md should be updated to include "[❔]" after the patch link.

When the script is run for the first time it removes newlines at the ends of the files, however, this doesn't change how the files are rendered on GitHub, an action that ran on my fork that showcases this can be found here. The action running for the first time after formatting has been applied via sed can be seen here. If needed, revisions in my fork of the wiki can provide more information about the script's effects.

Also, this is my first PR, so any criticism is welcome and appreciated.

Files

test-all-patches-action.yml

testPatches.sh

Emojis

pm4rcin commented 10 months ago

That's what I wanted to do for a long time but it's great that you did it. :)

  1. We have to wait for maintainers to tell if they are ok integrating it on that repo. If not then secondary repo could be created for that task.
  2. I think it should be improved by trying to compile dwl with make because sometimes old patches apply cleanly but some function has changed and it results with compilation errors. But it should be checked how much time it takes for all individual patches to compile.
  3. If 2 is going to be implemented the code should probably be modified to not compile every single instance of particular patch but only the newest one and maybe the one that was tagged for particular git tag e.g. 0.4 etc. But I would like to hear from more people what they think about this because I don't think we want too much complexity but IMO it's worth discussing at least.
  4. Verify that shellcheck is not complaining. I've pasted the script and there's output:
    
    $ shellcheck myscript

[Line 42:](javascript:setPosition(42, 13)) echo "$line" | sed "s/$emojiReplacePattern/[⚠️]/1" >> "$tempFile" ^-- SC2001 (style): See if you can use ${variable//search/replace} instead.

[Line 51:](javascript:setPosition(51, 13)) echo "$line" | sed "s/$emojiReplacePattern/[✅]/1" >> "$tempFile" ^-- SC2001 (style): See if you can use ${variable//search/replace} instead.

[Line 54:](javascript:setPosition(54, 13)) echo "$line" | sed "s/$emojiReplacePattern/[❌]/1" >> "$tempFile" ^-- SC2001 (style): See if you can use ${variable//search/replace} instead.


See if you can make it happy or if not then ignore it.
wyatt-avilla commented 10 months ago

Hey, thanks for the response. If more maintainers want to move forward with this, I'm happy to work on the other suggestions you made. For now though, I've fixed the shellcheck errors.

PalanixYT commented 10 months ago

I don't think this is a good idea. Not only is dwl trying to move away from Github, it also doesn't really matter because patches for tagged releases should work and patches for master usually don't apply because of a small change without implications for actual functionality

pm4rcin commented 10 months ago

@PalanixYT the yml file is small enough to change it to codeberg actions. As for patches for master it'd be an information if after some commit the patch has broken. Also it's not like most patches brake so often on master since a lot of code stays the same for a long time. Patches for tagged releases could be skipped as an option, just need to do some regex for it.

fauxmight commented 10 months ago

@PalanixYT has a very strong point against this automation. When patches fail to apply they require intervention. The intervention required is on the part of the patch maintainer(s). This is not something that should land on dwl core maintainer(s).

Trying to tack this onto core dwl gives users (particularly new users) the implication that dwl maintainer(s) are responsible for patches that fail to apply.

pm4rcin commented 10 months ago

@PalanixYT has a very strong point against this automation. When patches fail to apply they require intervention. The intervention required is on the part of the patch maintainer(s). This is not something that should land on dwl core maintainer(s).

Trying to tack this onto core dwl gives users (particularly new users) the implication that dwl maintainer(s) are responsible for patches that fail to apply.

Yeah I was thinking about the burden and on second thought what about a dedicated repo where users can create branches for master and for each tag? Let's say the name would something like dwl-contrib or dwl-patches or whatever you see fit.

sevz17 commented 9 months ago

Yeah, I'm not against this but I don't think this belongs to this repo.

wyatt-avilla commented 9 months ago

Hey everyone, thanks for the responses. A separate repo definitely seems like a better idea, and it's something I'll look into in the future.