Team 1 HW3 - Slack Chatbot Submission - #10
Conversation
Added TrelloManager
Signed-off-by: cindyke124 <152237584+cindyke124@users.noreply.github.com>
There was a problem hiding this comment.
Generally might want to ignore this (put in gitignore)
There was a problem hiding this comment.
Did you mean to delete most of the .gitignore? I think it's generally quite helpful
There was a problem hiding this comment.
It is bad practice to push secrets to the repository
| import re | ||
|
|
||
| # Initialize Slack and Trello clients | ||
| client = WebClient(token='xoxb-7855184757495-7855216149575-KI6lOoBXKSoudrc9pzugoW0x') |
There was a problem hiding this comment.
It is bad practice to hard-code your tokens. You should consider using os.getenv() as you have done below
| """Send a message to a Slack channel.""" | ||
| try: | ||
| response = client.chat_postMessage(channel=channel, text=message) | ||
| print("Message sent successfully:", response["message"]["text"]) |
There was a problem hiding this comment.
You can consider using the logger library instead of print statements
| } | ||
| response = requests.post(url, params=query, timeout=10) | ||
|
|
||
| if response.status_code == HTTP_OK: |
|
|
||
| return "" | ||
|
|
||
| def create_a_list(self, list_name: str, id_board: str) -> str: |
There was a problem hiding this comment.
Is create_list() taken? Generally articles are not included in function names.
There was a problem hiding this comment.
Can probably remove the dummy files we created in setup
There was a problem hiding this comment.
What is "refernciable" supposed to mean?
There was a problem hiding this comment.
Can probably remove these tests and replace them with ones that test your code's functionality
Alex-Ying
left a comment
Alex-Ying
left a comment
There was a problem hiding this comment.
Reviewed by:
Alex Ying - aty2009@nyu.edu
Mridul Mittal - mm13171@nyu.edu
kyotov
left a comment
kyotov
left a comment
There was a problem hiding this comment.
There are issues here, but mainly what I notice at first glance:
- How is each file in your PR connected to the task at hand?
- You should not have files that are irrelevant to the task.
- Make sure files that should be .gitignore are in .gitignore
- E.g. .DS_Store
- Make sure secrets are not checked in!
- Make sure there is a clear and separated API!
You should fix these before I look again.
Also Alex' comments are good -- make sure you take his feedback into account
| @@ -0,0 +1,54 @@ | |||
| version: 2.1 | |||
There was a problem hiding this comment.
how is this conneced to the chatbot?
|
The changes we have suggested in the earlier review havent been introduced |
@mittalmridul here is the link for the new PR to be reviewed, it was also in excel sheet: #14 |
7 participants
This pull request introduces enhancements to the Slack chatbot, improving message handling and integration. It addresses the initial setup requirements and lays the groundwork for future feature expansions.