-
-
Notifications
You must be signed in to change notification settings - Fork 89
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
whois command #352
whois command #352
Conversation
A regular user is probably not aware of the difference between user and member and will not understand what to expected from I would also turn around the logic, |
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
07fe77f
to
e19aaaa
Compare
Now I get it, sonarcloud is crying about the (unused) logger variable. |
a08060d
to
27c1506
Compare
8173ddf
to
6aa957a
Compare
Been 7 days, but due to the merge a new commit came (without changes) Can this be merged in the new few days? |
This PR has been stale for 12 days, could anyone review please? |
I wasnt aware its ready, sorry. |
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
Hm, I like it, but the default is a bit cluttered. Do we really need fields 'is bot', 'is booster', 'hypersquad'.. etc on each /whois we do? Maybe it makes more sense to have 2 commands, one concise and clear, so you can extract the most important info quickly, and another with more detailed info. Short one would be without banner/background image too, and that way it would be super space efficient, you can even use it in lobby, and it wouldn't be that spammy, since it's only few fields. |
You'll have to elaborate, what important info exactly? |
Basically just this: profile picture, ID, join/registration date. At least that's the only thing I use whois for. Personally, I would like to know user activity on the server as well, past month and overall. That would be the most useful field for me, alongside join/registration date. Since I often have to determine if the member is positively contributing in any way, or it's just a spambot. This would probably go under more detailed view tho. |
What does this mean?
Same for this, how can a bot determine this? |
Right, apologies for not being clear enough. Number of messages basically.
I would determine that, I just need bot to dig up some info for me. So use case goes like this:
But again, this is how and why I use |
Not possible, we could assume the number the tophelpers command retrieves.
Not possible
It seems the only info you want is
Now the question becomes, how do you want me to implement this?
But I personally don't really like it, it's becoming too much of an hassle to request data. Could you put your comment in #64 so it can be discussed further over there? |
@Tais993 Can you give Sonar some love? |
3915f25
to
6ac33cd
Compare
Closed to delete the branch temporary |
Kudos, SonarCloud Quality Gate passed! |
@Heatmanofurioso, @Zabuzard changes have been made, and Spotless has received their not so deserving attention |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See last open conversation
This pull request is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, it would look cleaner without the elses (what even is the plural of else?). Albeit, it's not required, and thus this review is not requesting changes.
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/commands/moderation/WhoIsCommand.java
Outdated
Show resolved
Hide resolved
bd244ee
to
d392fc2
Compare
Taking over. Ready to review. |
Kudos, SonarCloud Quality Gate passed! |
Open to suggestions for properties to add to the command
Implementation
I decided to go for the description instead of fields, fields are useful but fill your screen easily.
When adding a lot of them, it becomes harder to read and (imho) ugly.
The embed color goes as follows:
Member color -> profile color -> default (black)
So if it's an user, that has a custom banner it'll result in black.
But if it's a member, with a member color it'll show that color instead.
Current to-do's:
Command signature
/whois User:user (boolean:show_server_specific_info)
Member info
If the member is boosting the guild, it'll also display since when they started boosting.
If the member isn't in VC, it won't show the
in voicechannel
optionUser info
If the user has any flags (other than nitro and Hypesquad), it shows them