Avoid spamming #firemc #6

Closed
9pfs wants to merge 14 commits from 9pfs/FireBot:dont-spam-firemc into master
Collaborator

idk?

idk?
9pfs added 1 commit 2024-05-01 00:37:12 +00:00
9pfs requested review from Firepup650 2024-05-01 00:38:38 +00:00
9pfs added 1 commit 2024-05-01 00:44:06 +00:00
9pfs force-pushed dont-spam-firemc from d3a824dc24 to ce5c979e8d 2024-05-01 00:44:31 +00:00 Compare
Owner

You're gonna have to force update again @9pfs, I had to amend my last commit.

You're gonna have to force update again @9pfs, I had to amend my last commit.
Firepup650 added the
Kind/Enhancement
Priority
Zero
labels 2024-05-01 00:50:03 +00:00
Firepup650 requested changes 2024-05-01 00:52:21 +00:00
Dismissed
Firepup650 left a comment
Owner

I don't want an entire check in the primary bot code for this, if you're absolutely set on this, please add a QUIT handler to handlers.py, and then have it parse the QUIT line there. Then check the user (and host if you want to) who QUIT, and then I'll throw in a per-server check on it so we don't run this on random servers for no reason.

I don't want an entire check in the primary bot code for this, if you're absolutely set on this, please add a `QUIT` handler to `handlers.py`, and then have it parse the `QUIT` line there. Then check the user (and host if you want to) who `QUIT`, and then I'll throw in a per-server check on it so we don't run this on random servers for no reason.
9pfs was assigned by Firepup650 2024-05-01 00:53:14 +00:00
9pfs added 2 commits 2024-05-01 00:54:56 +00:00
9pfs added 1 commit 2024-05-01 00:56:03 +00:00
9pfs force-pushed dont-spam-firemc from ab865d3dbb to 9c5a305a02 2024-05-01 00:56:16 +00:00 Compare
Firepup650 requested changes 2024-05-01 00:56:47 +00:00
Dismissed
Owner

please remove this now

please remove this now
Author
Collaborator

That change was reverted by ab865d3dbb.

That change was reverted by ab865d3dbb.
Firepup650 marked this conversation as resolved
Owner

lmao i must have been in the middle of a review when you reverted

lmao i must have been in the middle of a review when you reverted
Author
Collaborator

Uh... the timer should be removed by this PR too, right?

Uh... the timer should be removed by this PR too, right?
Firepup650 requested changes 2024-05-01 00:58:29 +00:00
Dismissed
@ -175,0 +175,4 @@
def QUIT(bot: bare.bot, msg: str) -> tuple[None, None]:
if bot.server == "replirc":
if msg.split("!", 1)[0][1:] == "FireMCBot":
bot.send("TOPIC #firemc :FireMC Relay channel (offline)\n")
Owner

Last line of the function MUST be return None, None, it'll crash otherwise.

Last line of the function _**MUST**_ be `return None, None`, it'll crash otherwise.
Author
Collaborator

Fixing

Fixing
Firepup650 marked this conversation as resolved
Owner

Uh... the timer should be removed by this PR too, right?

Probably, make sure to remove it from the config as well. (also please bump the bot version up one)

> Uh... the timer should be removed by this PR too, right? Probably, make sure to remove it from the config as well. (also please bump the bot version up one)
9pfs added 2 commits 2024-05-01 01:00:49 +00:00
Owner

Reverting that specific commit is bad, there were other changes bundled in that commit that are critical.

Reverting that specific commit is bad, there were other changes bundled in that commit that are critical.
Firepup650 requested changes 2024-05-01 01:04:01 +00:00
Dismissed
bot.py Outdated
@ -271,3 +271,3 @@
for thread in self.threads:
tdict[thread] = timers.data[thread]
if tdict[thread]["passInstance"]:
if thread in ["radio"]:
Owner

Critical change, needs to be un-reverted

Critical change, needs to be un-reverted
Firepup650 marked this conversation as resolved
timers.py Outdated
@ -118,4 +117,3 @@
data: dict[str, dict[str, Any]] = {
"radio": {"noWrap": True, "func": radio, "passInstance": True},
Owner

related to my other commit, this needs to use the new passInstance param.

related to my other commit, this needs to use the new `passInstance` param.
Firepup650 marked this conversation as resolved
timers.py Outdated
@ -127,2 +120,2 @@
"ignoreErrors": True,
},
"radio": {"noWrap": True, "func": radio, "args": []},
"mc-down": {"noWrap": False, "func": mcDown, "args": [], "interval": 60, "ignoreErrors": True}
Owner

Reverting that commit didn't even remove the timer, lol

Reverting that commit didn't even remove the timer, lol
Author
Collaborator

Looking for other commits to revert then

Looking for other commits to revert then
Owner

Just revert the revert, I'll delete the stuff that needs removal myself if you want.

Just revert the revert, I'll delete the stuff that needs removal myself if you want.
Firepup650 marked this conversation as resolved
9pfs added 2 commits 2024-05-01 01:04:13 +00:00
9pfs added 1 commit 2024-05-01 01:06:58 +00:00
9pfs requested review from Firepup650 2024-05-01 01:07:14 +00:00
Firepup650 reviewed 2024-05-01 01:09:01 +00:00
bot.py Outdated
@ -271,3 +271,3 @@
for thread in self.threads:
tdict[thread] = timers.data[thread]
if tdict[thread]["passInstance"]:
if tdict[thread]["passInstance"]:
Owner

This needs to be indented another layer

This needs to be indented another layer
9pfs marked this conversation as resolved
timers.py Outdated
@ -126,3 +116,1 @@
"interval": 60,
"ignoreErrors": True,
},
"radio": {"noWrap": True, "func": radio, "args": []}
Owner

radio needs to use "passInstance": True, not "args": []

`radio` needs to use `"passInstance": True`, not `"args": []`
Firepup650 marked this conversation as resolved
9pfs added 1 commit 2024-05-01 01:11:17 +00:00
Firepup650 requested changes 2024-05-01 01:13:49 +00:00
Dismissed
bot.py Outdated
@ -271,3 +271,3 @@
for thread in self.threads:
tdict[thread] = timers.data[thread]
if tdict[thread]["passInstance"]:
if tdict[thread]["passInstance"]:
Owner

We CANNOT use a tab here, this will crash the bot. It absolutely must be 4 spaces.

We CANNOT use a tab here, this will crash the bot. It absolutely must be 4 spaces.
Author
Collaborator

Uh, what? I thought that WAS 4 spaces!

Uh, what? I thought that WAS 4 spaces!
9pfs marked this conversation as resolved
9pfs added 1 commit 2024-05-01 01:14:44 +00:00
9pfs added 1 commit 2024-05-01 01:17:16 +00:00
9pfs requested review from Firepup650 2024-05-01 01:17:54 +00:00
Firepup650 approved these changes 2024-05-01 01:20:08 +00:00
Owner

wtf does it mean no key.

wtf does it mean no key.
Author
Collaborator

I'm honestly shocked that you just approved this mess.

I'm honestly shocked that you just approved this mess.
Owner

do I have to give forejo a key or something?

do I have to give forejo a key or something?
Author
Collaborator

Maybe I'd have to add a key? idk, worst case scenario you could always merge, pull, git commit --amend --no-edit -S, and force push

Maybe I'd have to add a key? idk, worst case scenario you could always merge, pull, `git commit --amend --no-edit -S`, and force push
Owner

It literally won't let me merge it as-is, I'd have to override protection rules to merge this.

It literally won't let me merge it as-is, I'd have to override protection rules to merge this.
Author
Collaborator

I'll try to set up a key then, ig

I'll try to set up a key then, ig
Author
Collaborator

Never mind, idk how

Never mind, idk how
Owner
https://forgejo.org/docs/v1.19/admin/config-cheat-sheet/#repository---signing-repositorysigning I think?
Author
Collaborator

I just saw that, it makes absolutely no sense. Could you enable manual merge detection + do git remote add h ssh://h@git.h.hackclub.app:3322/9pfs/FireBot.git;git fetch --all;git switch master;git merge h/dont-spam-firemc;git commit --amend --no-edit -S to merge?

I just saw that, it makes absolutely no sense. Could you enable manual merge detection + do `git remote add h ssh://h@git.h.hackclub.app:3322/9pfs/FireBot.git;git fetch --all;git switch master;git merge h/dont-spam-firemc;git commit --amend --no-edit -S` to merge?
Owner

I just saw that, it makes absolutely no sense.

I think what that means is that you need a repository.signing MERGES key with something like approved as the value in app.ini.

> I just saw that, it makes absolutely no sense. I think what that means is that you need a `repository.signing` `MERGES` key with something like `approved` as the value in `app.ini`.
Owner
https://codeberg.org/forgejo/forgejo/src/branch/forgejo/custom/conf/app.example.ini#L1122-L1162
Author
Collaborator

This instance has no key to sign this commit with.

...

> This instance has no key to sign this commit with. ...
Author
Collaborator

Oh!!! I think I need to create a forgejo signing key + abuse BindPaths to make it get used or something.

Oh!!! I think I need to create a forgejo signing key + abuse BindPaths to make it get used or something.
Owner

There's an option at the top of the signing block to use a default key, why not just enable that?

There's an option at the top of the signing block to use a default key, why not just enable that?
Owner

GPG key to use to sign commits, Defaults to the default - that is the value of git config --get user.signingkey
run in the context of the RUN_USER

> GPG key to use to sign commits, Defaults to the default - that is the value of git config --get user.signingkey > run in the context of the RUN_USER
Firepup650 referenced this pull request from a commit 2024-05-01 01:47:14 +00:00
Owner

I merged this manually, for now. Closing PR since forejo is being silly.

I merged this manually, for now. Closing PR since forejo is being silly.
Firepup650 closed this pull request 2024-05-01 01:48:15 +00:00
Firepup650 referenced this pull request from a commit 2024-05-01 01:53:53 +00:00

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Firepup650/FireBot#6
No description provided.