search

zulip / python-zulip-api

Python library for the Zulip API.
https://zulip.com/api/
Apache License 2.0
359 stars 361 forks source link

IRC bridge: Remove "_zulip" nickname append. #616

Open rht opened 4 years ago

rht commented 4 years ago

The current behavior is to automatically append "_zulip" to user-specified nick prefix, hence forcing the bridge nick to end with "_zulip". This current behavior is confusing the user, especially if the user is using a password to connect to an IRC server. Additionally, if nickname is already in use, simply raise an error instead of silently adding more underscores to the nick.

Testing plan: I have tested this PR on my own laptop.

timabbott commented 4 years ago

How do we prevent mirroring loops without the _zulip suffix?

rht commented 4 years ago

How do we prevent mirroring loops without the _zulip suffix?

! I forgot about that. I suppose I can just exactly match the bot nickname.

rht commented 4 years ago

@timabbott added the fix in the 3rd commit.

rht commented 4 years ago

My testing worked because I was coincidentally using a nick that ends with "_zulip". Now it is no longer the case and still works.

rht commented 4 years ago

@timabbott comment addressed.

timabbott commented 4 years ago

Great; can you clean up the commit history?

(And I guess we'll want to document this, maybe as a first/preferred option above the Matrix version? Maybe just add a commit to your Slack docs PR and I can process them together)

rht commented 4 years ago

Commits squashed.

I looked at https://github.com/zulip/zulip/blob/master/templates/zerver/integrations/irc.md. It looks fine and doesn't recommend Matrix at all. Even the sentence at https://github.com/zulip/zulip/blob/master/templates/zerver/integrations/matrix.md recommends the direct IRC mirror.

rht commented 4 years ago

IMO, we should instead advice people to use Matterbridge, if they only want to mirror only to 1 topic in Zulip. I have seen the Matterbridge in action in julialang.zulipchat.com, and it mirrors multiple protocol with only 1 configuration file. The IRC bridge works, but with unavoidable error messages, that makes the script looks bad.

rht commented 3 years ago

@timabbott ping

zulipbot commented 3 years ago

Heads up @rht, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

rht commented 3 years ago

Rebased.