Skip to content

Infrastructure: Revert 6207 and 6277#6423

Merged
demonnic merged 5 commits intodevelopmentfrom
revert_6207_6277
Nov 14, 2022
Merged

Infrastructure: Revert 6207 and 6277#6423
demonnic merged 5 commits intodevelopmentfrom
revert_6207_6277

Conversation

@demonnic
Copy link
Copy Markdown
Member

@demonnic demonnic commented Nov 12, 2022

Brief overview of PR changes/additions

#6207 and subsequently #6277 have since been discovered to be the source of several bugs, and so we have decided to revert the changes in development but open a new PR for the changes themselves.

Motivation for adding to Mudlet

Other info (issues closed, discussion etc)

resolve the following issues:
#6416
#6394
#6321
#6290

@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Nov 12, 2022

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 12, 2022

Warnings
⚠️ PR makes changes to 11 source files. Double check the scope hasn't gotten out of hand

Generated by 🚫 dangerJS against 6141e92

Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like trying to revert those two PRs took out too much.

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source checks out on a visual scan 👍

@demonnic
Copy link
Copy Markdown
Member Author

I did check locally that the linked issues were resolved, but would love as many eyes on this as we can get

@demonnic
Copy link
Copy Markdown
Member Author

/refresh links

@vadi2 vadi2 added this to the 4.17.0 milestone Nov 14, 2022
Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks now like the code has properly been reverted - and it does compile - so I'm going to approve it. We will want to re-open the issues that were closed by those two PR - which seems to #5564 and which I have just re-opened...

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Nov 14, 2022

Let's merge it in and see how PTBs are like now

@demonnic
Copy link
Copy Markdown
Member Author

I will merge this over my lunch break, so I can be sure I have time to create the new PR to track trying to fix the issues with this fix/feature.

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Nov 14, 2022

thank you!

@demonnic demonnic merged commit 4a9e1a4 into development Nov 14, 2022
@demonnic demonnic deleted the revert_6207_6277 branch November 14, 2022 19:11
demonnic added a commit that referenced this pull request Nov 14, 2022
@demonnic
Copy link
Copy Markdown
Member Author

#6425 reverts the reversion done here so that work can be done on getting the code up to snuff.

@wiploo
Copy link
Copy Markdown
Contributor

wiploo commented Nov 15, 2022

#6290 is fixed by this revert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants