-
Notifications
You must be signed in to change notification settings - Fork 272
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.
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
Revert "Port both webhelper and markdown to webkit2gtk" #749
Conversation
Puh. I really hope I didn't mess it up again |
I'm not 100% sure how easy it is to reapply commits that have been reverted, short of making sure they are different commits (as otherwise they are already merged), so it might be slightly annoying for that other Markdown PR. Also, for WebHelper I don't really mind so long as the small problems gets fixed; I wanted to keep GTK2 support, but it's also true that in many recent distros (including next Debian) there is no more webkit1, so it's just not so useful anymore; and the delta between webkit1 and webki2 is bigger than a few GTK2/GTK3 conditionals. But either way is good, and it'd be just fine for me to work on a new PR if this is reverted. |
Not sure if I'm just confused, but wasn't it already merged? I honestly thought this revert had already happened as I checked the diff and #746 and #677 had no differences for Markdown, so I didn't think it would be meaningful to merge until this revert happened.
That was my issue with the Markdown changes, and it was somewhat fixed in follow ups, though I'd like to improve it to still support all of the versions of webkit it's known to work with (the versions/package names are confusing but I guess
IMO this is the best way, #677 should have never been made to change two plugins at once (especially plugins with different maintainers), I think that's where a lot of the confusion/problems came from. Maybe in the future we should just decline PRs which affect multiple plugins unless it's related to translations or build system or other more global stuff like that. |
@frlan AFAICT the merge of #746 happened, but not the this revert, see markdown history. This is still a pull request that is not merged. Not sure what is going to happen when this revert is applied, but as @codebrainz says, since #746 is a subset of #677, reverting #677 now may also revert #746. |
@frlan that's what I thought, but then what is this PR about? |
@codebrainz litter. Going to clean it up |
The PR was created by accident |
I think whats confusing is that fb82de4 doesn't show in the history of, for example, markdown/src/viewer.c. |
Reverts #677