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.

Sign up for GitHub

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

Jump to bottom

Add addusers command #27

Merged
merged 5 commits into from
Jul 29, 2023
Merged

Add addusers command #27

merged 5 commits into from
Jul 29, 2023

Conversation

J3y0
Copy link
Contributor

@J3y0 J3y0 commented Jul 11, 2023 •
edited
Loading

The command adds multiple users at once.

Linked to #17

@J3y0
Add addusers command
87834e6 
- The command adds multiple users at once
@ctmbl ctmbl mentioned this pull request Jul 11, 2023
Open
@J3y0
Fix(addusers): add request for adding user into the try catch
8a1aaaf 
- An exception for 404 response (if user_id is not valid) has
to be handled correctly -> a TODO has been added
- minor fix adding clearer logs
@ctmbl
Copy link
Contributor

ctmbl commented Jul 12, 2023

closes #17

@ctmbl ctmbl linked an issue Jul 12, 2023 that may be closed by this pull request
Add a addusers command #17
Closed
ctmbl
ctmbl reviewed Jul 12, 2023
View reviewed changes
Copy link
Contributor

@ctmbl ctmbl left a comment

Choose a reason for hiding this comment

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

Some minor adjustments but overall that's what we want!

It might be right to factorize/homogenize some code between adduser and addusers though

src/bot/root_pythia_cogs.py Outdated Show resolved Hide resolved
src/bot/root_pythia_cogs.py Outdated Show resolved Hide resolved
src/bot/root_pythia_cogs.py Outdated Show resolved Hide resolved
J3y0 added 2 commits July 12, 2023 23:06
atxr
atxr approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@atxr atxr left a comment

Choose a reason for hiding this comment

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

With this refactorization, this PR looks perfect!
You still need to resolve pylint issues! 🐞

@J3y0
Copy link
Contributor Author

J3y0 commented Jul 26, 2023

Sorry for that '^^

ctmbl
ctmbl approved these changes Jul 29, 2023
View reviewed changes
Copy link
Contributor

@ctmbl ctmbl left a comment •
edited
Loading

Choose a reason for hiding this comment

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

perfect!
tks @J3y0 you can merge it :)

@J3y0 J3y0 merged commit edbb5ff into iScsc:main Jul 29, 2023
@J3y0 J3y0 deleted the feat/addusers_command branch July 29, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ctmbl ctmbl ctmbl approved these changes

@atxr atxr atxr approved these changes

@ZynoXelek ZynoXelek Awaiting requested review from ZynoXelek

@FireFrozen94 FireFrozen94 Awaiting requested review from FireFrozen94

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a addusers command
3 participants
@J3y0 @ctmbl @atxr