Skip to content
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.

Sign up for GitHub

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

Jump to bottom

standard protection consequences #232

Merged
merged 6 commits into from
Mar 18, 2022
Merged

standard protection consequences #232

merged 6 commits into from
Mar 18, 2022
+228 −6

Conversation

jesopo
Copy link
Contributor

@jesopo jesopo commented Feb 21, 2022

No description provided.

@jesopo jesopo force-pushed the jess/standard-consequence branch 2 times, most recently from 7d9fe14 to 27706e8 Compare February 21, 2022 17:07
jesopo
jesopo commented Feb 21, 2022
View reviewed changes
src/Mjolnir.ts Outdated Show resolved Hide resolved
@jesopo jesopo changed the title standard/undoable protection consequences standard protection consequences Feb 22, 2022
@jesopo jesopo requested a review from Yoric February 22, 2022 13:53
Yoric
Yoric reviewed Feb 22, 2022
View reviewed changes
src/protections/consequence.ts Outdated Show resolved Hide resolved
src/protections/consequence.ts Outdated Show resolved Hide resolved
src/Mjolnir.ts Outdated Show resolved Hide resolved
src/Mjolnir.ts Outdated Show resolved Hide resolved
src/Mjolnir.ts Outdated Show resolved Hide resolved
src/Mjolnir.ts Outdated Show resolved Hide resolved
src/Mjolnir.ts Show resolved Hide resolved
@rubo77
Copy link

rubo77 commented Feb 23, 2022

What is this this pull request about?

@Yoric
Copy link
Contributor

Yoric commented Feb 23, 2022 •
edited
Loading

It's a refactoring to:

  1. make further work on protections easier;
  2. make it easier to plug other bots who may decide to act when a Mjölnir protection has been triggered — we're generally trying to make it easier to plug other bots into Mjölnir, not just with protections.

@jesopo
Copy link
Contributor Author

jesopo commented Feb 23, 2022

@Gnuxie I think the test failure here isn't my doing? unsure

@Gnuxie
Copy link
Contributor

Gnuxie commented Feb 23, 2022

@Gnuxie I think the test failure here isn't my doing? unsure

Yeah, this is been happening in a few places it seems, it doesn't seem to be very lenient on CI, i'll change it

@jesopo jesopo requested a review from Yoric February 23, 2022 15:50
Yoric
Yoric reviewed Feb 24, 2022
View reviewed changes
src/Mjolnir.ts Outdated Show resolved Hide resolved
src/Mjolnir.ts Outdated Show resolved Hide resolved
src/protections/IProtection.ts Outdated Show resolved Hide resolved
src/protections/IProtection.ts Outdated Show resolved Hide resolved
src/protections/consequence.ts Outdated Show resolved Hide resolved
src/protections/consequence.ts Outdated Show resolved Hide resolved
test/integration/standardConsequenceTest.ts Show resolved Hide resolved
test/integration/standardConsequenceTest.ts Show resolved Hide resolved
@jesopo jesopo force-pushed the jess/standard-consequence branch from 48d5208 to 2b60f6a Compare March 4, 2022 14:46
@jesopo jesopo requested a review from Yoric March 7, 2022 14:50
Yoric
Yoric reviewed Mar 7, 2022
View reviewed changes
src/Mjolnir.ts Show resolved Hide resolved
src/Mjolnir.ts Show resolved Hide resolved
src/protections/consequence.ts Show resolved Hide resolved
test/integration/standardConsequenceTest.ts Show resolved Hide resolved
@jesopo jesopo requested a review from Yoric March 15, 2022 13:19
Yoric
Yoric approved these changes Mar 17, 2022
View reviewed changes
Copy link
Contributor

@Yoric Yoric left a comment

Choose a reason for hiding this comment

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

Looks good!

Apologies for the delay, I'm still a bit sick.

src/Mjolnir.ts Show resolved Hide resolved
src/protections/consequence.ts Show resolved Hide resolved
@jesopo jesopo merged commit 1880287 into main Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yoric Yoric Yoric approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jesopo @rubo77 @Yoric @Gnuxie