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.

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

Implement draft/message-redaction #2065

Merged
merged 10 commits into from
May 31, 2023
Merged

Implement draft/message-redaction #2065

merged 10 commits into from
May 31, 2023

Conversation

progval
Copy link
Contributor

@progval progval commented May 26, 2023

Permission to use REDACT mirrors permission for 'HistServ DELETE'

Spec: ircv3/ircv3-specifications#524 (this is a variant of ircv3/ircv3-specifications#425 with only deletion/redaction, and targetmsg moved from a tag to a parameter)

Tests: progval/irctest#203

@slingamn Thoughts on this design before I add support for PMs?

Val Lorentz added 2 commits May 26, 2023 16:12
Permission to use REDACT mirrors permission for 'HistServ DELETE'
Copy link
Member

@slingamn slingamn left a comment

Choose a reason for hiding this comment

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

This is legit! It's clean and isolated and doesn't commit Ergo to any particular position in the ongoing debate.

I'd say go ahead.

// length of the uint32 array that represents the bitset:
bitsetLen = 1
bitsetLen = 2
Copy link
Member

Choose a reason for hiding this comment

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

🎉

Makefile Outdated
go install -v -ldflags "-X main.commit=$(GIT_COMMIT) -X main.version=$(GIT_TAG)"

build:
build: ${capdef_file}
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather we didn't, this just slows down the build in the normal case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only a stat() call...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Make won't rebuild a target if a file with the same name already exists and is more recent than its dependencies)

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, I normally don't use this feature of make (I normally do 100% phony targets). I would prefer if we didn't introduce it because I don't understand it very well :-|

irc/histserv.go Outdated Show resolved Hide resolved
Val Lorentz added 5 commits May 29, 2023 13:54
slingamn says it's probably not desirable, and I'm on the fence.
Out of laziness, let's omit it for now, as it's not a regression
compared to '/msg HistServ DELETE'.
@progval progval marked this pull request as ready for review May 29, 2023 19:00
Copy link
Member

@slingamn slingamn left a comment

Choose a reason for hiding this comment

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

This looks good, I'll merge it if you undo the Makefile changes?

Comment on lines +2729 to +2730
// If this is a PM, we just removed the message from the buffer of the other party;
// now we have to remove it from the buffer of the client who sent the REDACT command
Copy link
Member

Choose a reason for hiding this comment

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

Good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants