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

[Module/idlemove] Store user's original channel (fixes #13) #14

Closed
wants to merge 1 commit into from

Conversation

raku-cat
Copy link
Contributor

I haven't tested this all too much and it might be too dirty hackish, but it does seem to work.

@azlux
Copy link
Contributor

azlux commented Sep 8, 2020

dirty but good.

@Kissaki Kissaki force-pushed the master branch 3 times, most recently from 7d19720 to 1f36911 Compare March 11, 2024 23:42
Copy link
Member

@Kissaki Kissaki left a comment

Choose a reason for hiding this comment

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

review notes:

affectedusers type dict, key: server id, value: set "index" of user session ids

on update the session id is added and removed to index respectively


I think it would make more sense to store the origchan with the session id, in the same structure, instead of with a different type of key and separate to it.

While this works, I don't think it's very maintainable. The key mismatch is confusing, and what we want to remember together is not together.


The PR source branch repository has been archived, so I can't push a rebase conflict resolution.

Python and this long idlemove type is a hassle and barrier to me.

I would

  1. Change affectedusers type from set to dict, with key user session id and value orig channel id

@Kissaki
Copy link
Member

Kissaki commented Mar 15, 2024

Migrated to PR #14

@Kissaki Kissaki closed this Mar 15, 2024
Kissaki added a commit that referenced this pull request Mar 15, 2024
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.

3 participants