In the end I reused the same PR (unintentionally, I just pushed to the same branch..). I'm happy for council to delay to next week. Especially since MattJ had some more stuff to add?
MattJ
I might figure out a sentence or two about pubsub#itemreply
pep.
Okay
pep.
I tried to split commits to make the review slightly easier. I id change existing normative text with my publish_node_full change so ~~. Also feedback on language / words / style etc. is welcome
Wojtekhas joined
gooyahas joined
sonnyhas left
sonnyhas joined
MattJ
I put a PR in your PR: https://github.com/Ppjet6/xeps/pull/1
MattJ
Reviewing your changes, everything looks good, except it feels a little confusing when it says: "the service MUST delete one of the existing items"
MattJ
This MUST is not conditional on the value pubsub#publish_node_full, but it's obviously intended to be
MattJ
Hmm, I think the simplest fix may be to simply append "or reject the new item." - all the cases are explained in the following text, so it should be clear enough
MattJ
I'll add it to my PR
pep.
hmm you aven't included it yet right? I merged the PR and I read the end of your messages right after :/
pep.
I can change that myself, let me find the place
pep.
Ah right
pep.
I guess I didn't want to change the original sentence too much so that the normative parts don't change, but..
pep.
A pubsub service still needs to do all of the original text even though it doesn't implement publish-node-full
pep.
"and the maximum is reached when an item is published" this looked slightly confusing to me, but that's the original wording and it seems people understood it ok? So I'm not gonna change it
MattJ
Ah, I just committed https://github.com/Ppjet6/xeps/commit/905c18f4f2384394dab181c6ac1a406653ef55e7
MattJ
Okay, I hadn't really thought about the existing text
MattJ
It contradicts itself? :P
MattJ
One section says "the service MUST delete one of the existing items" and the other says "the service MUST return a &conflict; error"
MattJ
Sorry, I didn't realise this inconsistency was already there
bsqjxdhas left
bsqjxdhas joined
MattJ
Ah no, that is indeed new text
pep.
Ah sorry I just pushed something, I hadn't noticed activity in this rom :x✎
pep.
Ah sorry I just pushed something, I hadn't noticed activity in this room :x ✏
pep.
(many unread rooms)
pep.
Well.. the "MUST return a &conflict; error" is only if the option is set to "reject"
pep.
let me try to pull your changes nonetheless
MattJ
Yeah, I adjusted the order of clauses in that commit I just linked, to make that clearer (just in case...)
pep.
How did you link to a commit on my repo I haven't merged?
Now I just regret writing "any of the following values" instead of "one of the following values" (but I don't want to make merging harder)
pep.
I may be able to edit that, it's fine
bsqjxdhas left
bsqjxdhas joined
pep.
Ok I've pushed our changes
vaulorhas left
jonas’
o/ due to lack of an agenda and facing conflicting private appointments, I decided to prioritise the latter and will likely not be available unless the meeting runs for >20mins
jonas’
sorry for the short notice
vaulorhas joined
sonnyhas left
sonnyhas joined
daniel
no worries. I was just going to encourage everyone to catch up with https://mail.jabber.org/pipermail/standards/2023-March/039215.html and the corresponding PR (so we can vote on it next week) and pretty much leave it at that