borogove

created pr with borogove/muc-create.1 on 2026-10-28T20:49:38Z · by 9e834e0d

added borogove/muc-create.2 on 2026-10-30T12:26:53Z · by 3c31bc8c


                      1:  7eacfb3 < -:  ------- Initial channel creation


                      2:  4b5a982 ! 1:  d85d8d1 review


                      3:  8c9556b < -:  ------- Support intant room creation


                      4:  941aad7 < -:  ------- Add tests

added borogove/muc-create.3 on 2026-11-03T03:40:00Z · by 9e834e0d


                      -:  ------- > 1:  0aa766b Initial channel creation


                      1:  d85d8d1 ! 2:  a7f33d5 After config form is submitted, creation is complete


                      -:  ------- > 3:  e6042b0 Support intant room creation


                      -:  ------- > 4:  366bac7 Add tests


                      -:  ------- > 5:  fece298 review


                      -:  ------- > 6:  322b2bd Review replies

added borogove/muc-create.4 on 2026-11-03T03:42:40Z · by 9e834e0d


                      1:  0aa766b = 1:  0aa766b Initial channel creation


                      2:  a7f33d5 = 2:  a7f33d5 After config form is submitted, creation is complete


                      3:  e6042b0 = 3:  e6042b0 Support intant room creation


                      4:  366bac7 ! 4:  0da1c7e Add tests


                      5:  fece298 = 5:  86bad8e review


                      6:  322b2bd = 6:  997cf5a Review replies

commented on 2026-11-09T14:24:06Z · by 3c31bc8c

lgtm!

cmds

checkout latest patchset:

ssh pr.pico.sh pull borogove:muc-create | git am -3

checkout specific patchset revision:

ssh pr.pico.sh pull borogove:muc-create [rev] | git am -3

add changes to patch request:

git format-patch main --stdout | ssh pr.pico.sh borogove:muc-create

add comment to patch request:

ssh pr.pico.sh comment borogove:muc-create "lgtm!"

Patchset `borogove/muc-create.2` on 2026-10-30T12:26:53Z · commit d85d8d1

→ review

Eric Roberts 2026-10-30T12:26:06Z

review

Eric Roberts 2026-10-30T12:26:06Z

Semantic diff summary

0 added, 0 modified, 0 signature changed, 0 removed across 0 analyzed files (2 files skipped: unsupported file type)

+8 -0 borogove/Chat.hx #

......@@ -1640,6 +1640,7 @@ class Channel extends Chat {
					return stanza;
				}
			);
			// eric: does syncing actually cause a problem or is this just an optimization?
			if (sync) persistence.syncPoint(client.accountId(), chatId).then((point) -> doSync(point));

			return null;

......@@ -1909,6 +1910,10 @@ class Channel extends Chat {

	@:allow(borogove)
	private function setPresence(resource:String, presence:Presence, noStore:Bool) {
		// eric: we make this and then add it to a bunch of branches. I think it'd be nicer if we
		// could establish some clearer paths through, like if uiState == creating { do this } else
		// { do this } and they could call shared functions inside so it's clearer which operations
		// happen in each branch
		final wasCreating = uiState == Creating;
		final oneTen = presence?.mucUser?.statusCodes?.find((status) -> status == "110");
		final member = buildMember(resource, presence);

......@@ -1940,6 +1945,9 @@ class Channel extends Chat {
			}
			if (mav?.attr?.get("until") != null && mavUntil != mav?.attr?.get("until")) {
				mavUntil = mav?.attr?.get("until");
				// eric: if we're creating we could skip this whole outer if I think? possibly an
				// example of something that could be a separate function that the creating branch
				// doesn't call
				if (!noStore && !wasCreating) persistence.storeChats(client.accountId(), [this]);
			}
			if (!noStore && !wasCreating && mucUser.role != "visitor" && mucUser.jid != null) {

+7 -0 borogove/Client.hx #

......@@ -2684,6 +2684,8 @@ class Client extends EventEmitter {

	private function pingAllChannels(refresh: Bool) {
		for (channel in getChannels()) {
			// eric: I think you already put an early return inside of selfPing that returns early,
			// so don't need this one.
			if (channel.uiState == Creating) continue;
			channel.selfPing(refresh || channel.disco == null);
		}

......@@ -2692,6 +2694,11 @@ class Client extends EventEmitter {
	private function joinAllChannels() {
		channelPinger.clear();
		for (channel in getChannels()) {
			// eric: maybe a function on channel for this that returns early?
			//
			// eric: though with all the different behaviour between the two i'm starting to think
			// maybe a CreatingChannel should be its own type that can turn into a Channel on
			// success
			if (channel.uiState == Creating) continue;
			if (channel.disco.identities.length < 1) {
				channel.refreshDisco(() -> {

...	...		@@ -1640,6 +1640,7 @@ class Channel extends Chat {
1640	1640		return stanza;
1641	1641		}
1642	1642		);
	1643	+	// eric: does syncing actually cause a problem or is this just an optimization?
1643	1644		if (sync) persistence.syncPoint(client.accountId(), chatId).then((point) -> doSync(point));
1644	1645
1645	1646		return null;

...	...		@@ -2684,6 +2684,8 @@ class Client extends EventEmitter {
2684	2684
2685	2685		private function pingAllChannels(refresh: Bool) {
2686	2686		for (channel in getChannels()) {
	2687	+	// eric: I think you already put an early return inside of selfPing that returns early,
	2688	+	// so don't need this one.
2687	2689		if (channel.uiState == Creating) continue;
2688	2690		channel.selfPing(refresh \|\| channel.disco == null);
2689	2691		}

...	...		@@ -1909,6 +1910,10 @@ class Channel extends Chat {
1909	1910
1910	1911		@:allow(borogove)
1911	1912		private function setPresence(resource:String, presence:Presence, noStore:Bool) {
	1913	+	// eric: we make this and then add it to a bunch of branches. I think it'd be nicer if we
	1914	+	// could establish some clearer paths through, like if uiState == creating { do this } else
	1915	+	// { do this } and they could call shared functions inside so it's clearer which operations
	1916	+	// happen in each branch
1912	1917		final wasCreating = uiState == Creating;
1913	1918		final oneTen = presence?.mucUser?.statusCodes?.find((status) -> status == "110");
1914	1919		final member = buildMember(resource, presence);

...	...		@@ -1940,6 +1945,9 @@ class Channel extends Chat {
1940	1945		}
1941	1946		if (mav?.attr?.get("until") != null && mavUntil != mav?.attr?.get("until")) {
1942	1947		mavUntil = mav?.attr?.get("until");
	1948	+	// eric: if we're creating we could skip this whole outer if I think? possibly an
	1949	+	// example of something that could be a separate function that the creating branch
	1950	+	// doesn't call
1943	1951		if (!noStore && !wasCreating) persistence.storeChats(client.accountId(), [this]);
1944	1952		}
1945	1953		if (!noStore && !wasCreating && mucUser.role != "visitor" && mucUser.jid != null) {

...	...		@@ -2692,6 +2694,11 @@ class Client extends EventEmitter {
2692	2694		private function joinAllChannels() {
2693	2695		channelPinger.clear();
2694	2696		for (channel in getChannels()) {
	2697	+	// eric: maybe a function on channel for this that returns early?
	2698	+	//
	2699	+	// eric: though with all the different behaviour between the two i'm starting to think
	2700	+	// maybe a CreatingChannel should be its own type that can turn into a Channel on
	2701	+	// success
2695	2702		if (channel.uiState == Creating) continue;
2696	2703		if (channel.disco.identities.length < 1) {
2697	2704		channel.refreshDisco(() -> {

borogove

Patchset borogove/muc-create.2 on 2026-10-30T12:26:53Z · commit d85d8d1

Patchset `borogove/muc-create.2` on 2026-10-30T12:26:53Z · commit d85d8d1