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

Refactor front-end. Add support for discord.ext, command_config.json and feedback label. #5

Merged
merged 2 commits into from
May 22, 2020

Conversation

diivm
Copy link
Contributor

@diivm diivm commented Apr 26, 2020

solves #nothing 😆

main.py Outdated Show resolved Hide resolved
@diivm
Copy link
Contributor Author

diivm commented Apr 27, 2020

@kunaltyagi

@kunaltyagi
Copy link
Member

The code is quite similar with review_q having difference in only title_suffix and label. Could be refactored a bit more to reduce duplication

main.py Outdated Show resolved Hide resolved
@diivm
Copy link
Contributor Author

diivm commented May 2, 2020

825645a: How does this class look?

discord-bot/main.py

Lines 189 to 273 in 825645a

class SendReply:
command_type=None
channel=None
number_of_issues=None
author=None
git_config=None
discord_config=None
issues=[]
reply=None
chosen_issues=[]
def __init__(self, command_type, channel, number_of_issues, author):
self.command_type=f"{command_type}"
self.channel=channel
self.number_of_issues=number_of_issues
self.author=author
command_config=read_config(command_config_filepath)
self.git_config=command_config["type"][command_type]["git"]
self.discord_config=command_config["type"][command_type]["discord"]
def issues_generator(self):
self.issues=[x async for x in get_issues(self.git_config["repository"], self.git_config["closed"],
self.git_config["pull_request"],
self.git_config["include_labels"], self.git_config["exclude_labels"],
self.git_config["sort"], self.git_config["ascending_order"],
self.git_config["error_channel"])]
def reply_message(self, chosen_issues):
self.reply = discord.Embed(color=discord.Color.purple())
self.reply.title = self.discord_config["title"]
if len(self.chosen_issues) < self.number_of_issues:
self.reply.set_footer(text="There weren't enough...")
self.reply.description = compose_message(beautify_issues(self.chosen_issues))
async def reply_handler(self, sorter):
async def inner_handler(self):
await set_playing(self.discord_config["game_name"])
error_channel = error_handler(self.discord_config["error_channel"])
async with channel.typing():
issues_generator()
sorter()
reply_message()
await channel.send(embed=reply)
await set_playing('The Waiting Game')
return inner_handler
@reply_handler
async def give_random(self):
"""
@TODO: improve algorithm to save queries
1. get first batch of github_max (100), find total number
2. if total < requested, return all
3. Generate requested random numbers
4. get issues till max(generated_list)
5. return them
"""
self.chosen_issues=random.choices(issues, k=number_of_issues)
@get_reply
async def review_q(self):
if author:
self.issues = pr_with_pending_review(get_pr_details(self.issues), self.author)
# since async islice doesn't exist
async for issue in self.issues:
self.chosen_issues.append(issue)
if len(self.chosen_issues) == self.number_of_issues:
break
else:
async for issue in self.issues:
self.chosen_issues.append(issue)
if len(self.chosen_issues) == self.number_of_issues:
break
@get_reply
async def feedback_q(self):
# no author check needed
async for issue in self.issues:
self.chosen_issues.append(issue)
if len(self.chosen_issues) == self.number_of_issues:
break

If it is good, I'll take that and refactor some more later in the file:

discord-bot/main.py

Lines 316 to 379 in 825645a

elif command == "rand":
if len(args) != 1:
await channel.send(embed=reply)
return
try:
number_of_issues = int(args[0].strip())
if number_of_issues < 1:
raise ValueError("Positive integer needed")
except ValueError:
reply.description = "I can't give you un-natural issues." + \
" I'm not a monster!!"
await channel.send(embed=reply)
return
if number_of_issues > 10:
number_of_issues = 10
reply.description = "Let's curb that enthusiasm.. just a little"
await channel.send(embed=reply)
await get_reply(channel=channel, number_of_issues=number_of_issues, author=None, get_params(command))
return
elif command == "rq" or command == "review":
if len(args) != 1:
await channel.send(embed=reply)
return
try:
number_of_issues = int(args[0].strip())
if number_of_issues < 1:
raise ValueError("Positive integer needed")
except ValueError:
await channel.send("This queue is 100% natural. Check your orders")
return
if number_of_issues > 10:
number_of_issues = 10
reply.description = "Let's curb that enthusiasm.. just a little"
await channel.send(embed=reply)
author = message.author
if command == "q":
author = None
elif isinstance(author, discord.Member):
author = author.nick or author.name
elif isinstance(author, discord.User):
author = author.name
await get_reply(channel=channel, number_of_issues=number_of_issues, author=author, get_params(command))
return
elif command == "fq" or command == "feedback":
if len(args) != 1:
await channel.send(embed=reply)
return
try:
number_of_issues = int(args[0].strip())
if number_of_issues < 1:
raise ValueError("Positive integer needed")
except ValueError:
await channel.send("This queue is 100% natural. Check your orders")
return
if number_of_issues > 10:
number_of_issues = 10
reply.description = "Let's curb that enthusiasm.. just a little"
await channel.send(embed=reply)
await get_reply(channel=channel, number_of_issues=number_of_issues, author=None, get_params(command))
return
return

command_config.json Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
command_config.json Outdated Show resolved Hide resolved
command_config.json Outdated Show resolved Hide resolved
command_config.json Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
@diivm diivm marked this pull request as draft May 6, 2020 16:11
@kunaltyagi
Copy link
Member

What we currently need/use the bot for:

  • Random issues for cleanup

  • feedback label:

    • issue
    • PR
  • code review label:

    • general
    • my code review
  • Reorganize the front end to have three nouns (rand, feedback, review)

  • Add a classifier to the noun (issue/pr) so it becomes: rand 5 pr, review 4, feedback 3 issue

    • default to issue for random and feedback
    • not for review, since review is for PR only
    • autocomplete using substring match (issue, issues, is, pr, pull, pulls, pull-requests)
  • Either add a classifier to review (general/mine) or just get both the data (general, and sorted by author's name, validated by API. Eg: for me)

This keeps the commands to a manageable number, reduces the paths needed for current requirement.

@diivm diivm marked this pull request as ready for review May 11, 2020 21:34
@diivm
Copy link
Contributor Author

diivm commented May 11, 2020

On top of my head:

  • Not done classes for commands as they are good only if we use cogs
  • decorators were creating complexity in understanding the flow of code, would much rather use fn calls to choose issues
    • oneshot requires random sampler, so handling everything in a @bot.command would not have worked since params are needed.
  • removed discord config, was unnecessary
  • added noun support, it modifies config argument according to noun
    • issue: pull_request=False
    • PR: pull_request=True, for the given config

Basically I eliminated message event and its argument parsing, and used commands instead, meanwhile converting repeating code into checks and functions, on top of the config changes.

That's all I can remember, it's 03:15 😪

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

partial review. Already looks a lot better 😄

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
command_config.json Outdated Show resolved Hide resolved
@diivm
Copy link
Contributor Author

diivm commented May 12, 2020

@kunaltyagi
I would look into the suggestions after the current code (corrected for silly errors) work for the testing application I am running.
Some of the changes you mentioned have already been addressed while debugging.

Current status:
After making some changes, the one-shot mode is running fine but the choose_issues function is not working as expected (returning There weren't enough always, i.e. choose_issues list is not properly being formed)
Also some weird error about async for is invalid syntax. Probably some version issue, it's a keyword maybe.

After it gets working, I'll address the remaining changes.

Edit: async for worked, one-shot worked

@kunaltyagi kunaltyagi marked this pull request as draft May 12, 2020 11:33
@kunaltyagi
Copy link
Member

I've made this a draft. Ping me when you think I should take a look 😄

main.py Outdated Show resolved Hide resolved
@diivm
Copy link
Contributor Author

diivm commented May 12, 2020

@kunaltyagi
It's running fine as of this moment 😆
Ready for further discussion, I guess.

  • Clean rebase after final approval
  • Complete formatting using black in new PR.

@diivm diivm marked this pull request as ready for review May 12, 2020 17:18
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
@diivm diivm force-pushed the add_feedback branch 3 times, most recently from 22006f0 to f3ced82 Compare May 15, 2020 15:08
@diivm
Copy link
Contributor Author

diivm commented May 15, 2020

I guess it's enough for this PR, more for the bot later.
Let me know if anything to be done prior to merge, apart from formatting.

@kunaltyagi
Copy link
Member

Can you update the PR description?

@diivm diivm changed the title add support for feedback label Refactor front-end, use discord.ext.commands, add feedback label May 15, 2020
@diivm diivm changed the title Refactor front-end, use discord.ext.commands, add feedback label Refactor front-end. Add support for discord.ext, command_config.json and feedback label. May 15, 2020
@diivm
Copy link
Contributor Author

diivm commented May 16, 2020

@kunaltyagi
Should I format using black in this PR itself?

Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

I'd suggest black here, because you're touching almost all lines anyways. Another option:

  • Separate PR for black, let that merge first
  • Same config here

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

Looks very good to me

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
@kunaltyagi
Copy link
Member

Squash as needed and ping back. Thanks for the effort you've put in this @divmadan

@diivm
Copy link
Contributor Author

diivm commented May 16, 2020

@kunaltyagi

  1. Format main.py using black #8
  2. Refactor front-end. Add support for discord.ext, command_config.json and feedback label. #5 (review)
  3. 🚀

Thanks for all the help!

@kunaltyagi
Copy link
Member

Conflicts here

@diivm
Copy link
Contributor Author

diivm commented May 18, 2020

Good to go!

@diivm diivm requested a review from kunaltyagi May 18, 2020 18:22
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

I've not testing this locally, but the code (in theory) looks good. Will be back after testing.

diivm added 2 commits May 22, 2020 15:24
- add command support from discord.ext
- add feedback support
- add noun support
@kunaltyagi kunaltyagi merged commit 55dd451 into PointCloudLibrary:master May 22, 2020
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