Hw3 submission #11

isiddharthsingh · 2024-12-07T04:44:10Z

-> How is each file in your PR connected to the task at hand?

Refactored the codes in folder src and follow the concept of api to have central communication.
Created tests for all the connections from slack, gmail, calendar, trello.
Updated check for ruff
Have a centralized app.py file that run the application and communication with the bot.
Updated circleCi file to use uv instead of pip.
Python versions have been updated to use 3.12

-> You should not have files that are irrelevant to the task.

No irrelevant files are there as of now.

-> Make sure files that should be .gitignore are inin .gitignore
E.g. .DS_Store*

All local/environment files are in .gitignore

-> Make sure secrets are not checked in!

We have ensured that no secrets are checked in

-> Make sure there is a clear and separated API!

We have clearly separated our APIs and follow the src/api directory structure

-> If your change is big and changes many files a description of the change overall is in order
-> How should it be reviewed?

I refactored all the codes to be in src/ files and now everything is separate like slack, gmail, calendar, trello.
Tests have been added and all the test are now passed.

-> What should the reviewer look at in what order?

All codes are in src folder.
All tests are in tests folder.
-> What is the main idea?
When the app is run, the user can open slack and ask to add stuff to trello list, remove it, delete the whole list, remove specific cards from list, it also fetches emails and commands like fetch emails, fetch 10 emails are there. User also has functionality to add deadlines to trello and that deadline is also synced with google calendar. if it is deleted from trello then the task is also deleted from calendar.
Many more functions like summarizing mails and adding to trello are in progress.

trello integration with show mw toto and add todo comands on slack Add .DS_Store to .gitignore Add correct python version and uv Add correct python version Add correct python version and checks fix: correct python version and enbled all ruff checks Update Readme.md Update CircleCi with uv package manager Added uv.lock Update README with uv dependency installation instructions Added .DS_Store to ignored files trello integration with show mw toto and add todo comands on slack Add .DS_Store to .gitignore Add correct python version Add correct python version and checks fix: correct python version and enbled all ruff checks Update Readme.md Update CircleCi with uv package manager

Sidhved · 2024-12-16T03:36:30Z

1. Change Granularity
Observation: Too many features in a single PR. This team focuses on Trello management, so the Slackbot and Gmail modules are irrelevant features and should not be contained in this PR. We suggest creating new PRs for those features respectively instead of including them in this PR.

2. Code Functionality
The module achieves its goal of enabling interaction with the Trello API. It is well segmented and documented with appropriate docstrings for descriptions.
We suggest adding files like .python-version in .gitignore file.

3. Readability & Structure

Naming Conventions: Function names are descriptive and follow snake_case, which is Pythonic.
Some suggestions for variable names are pointed in the corresponding locations in the code wherever needed.
Modularity: trello_bot.py directly depends on trello_api.py without any intermediate abstraction, making it tightly coupled. This coupling could lead to maintenance issues if the API functions are modified or extended. Encapsulating bot logic in a class and separating mapping-related logic into its own module would enhance modularity and scalability.

4. Standards Compatibility

Linting: The code largely follows PEP 8 guidelines, but there are minor inconsistencies:
Line spacing is inconsistent in some parts. Comments should be preceded by a single blank line to enhance readability.
API Exposure: API Exposure: API keys and other sensitive information are handled securely, which is good. Using environment variables is a good practice, and the code seems to follow this approach.

5. Testing

Coverage: Test coverage is good. However, you also should not contain the test files for other features such as Slackbot and Gmail in this PR.
Readability: The test cases are easy to read and follow. Adding some comments would be better for understanding the purpose and input/output.

Reviewers: Sidhved Warik (sw6071), Yaxin Ke (yk2991)

An open-ended question on the choice of using a separate requirements.txt for dependencies when project. toml made by uv already exists? And can the Operating System related dependencies be mentioned separately?

cindyke124 · 2024-12-16T03:37:07Z

src/trello/trello_api.py

+    """
+    # Fetch all lists (both open and closed if include_archived is True)
+    filter_value = 'all' if include_archived else 'open'
+    url_all_lists = f"https://api.trello.com/1/boards/{TRELLO_BOARD_ID}/lists"


Trello API base URLs (https://api.trello.com) are repeated multiple times. This could lead to errors if the API version changes. Consider defining a constant like TRELLO_API_BASE = "https://api.trello.com/1".

This has been fixed in the new version.

cindyke124 · 2024-12-16T03:42:45Z

src/trello/trello_api.py

+        return True
+    else:
+        raise Exception(f"Failed to archive the list '{list_name}': {response.status_code}, {response.text}")
+


A consistent number of blank lines should be used.

This has been fixed in the new version.

cindyke124 · 2024-12-16T03:43:46Z

src/trello/trello_api.py

+    # Fetch all lists (both open and closed if include_archived is True)
+    filter_value = 'all' if include_archived else 'open'
+    url_all_lists = f"https://api.trello.com/1/boards/{TRELLO_BOARD_ID}/lists"
+    query_params = {


query and query_params are inconsistently used. Standardize the naming for clarity.

This has been fixed in the new version.

cindyke124 · 2024-12-16T03:46:43Z

src/trello/trello_api.py

+        # List does not exist, so create it
+        list_id = create_trello_list(list_name)
+
+    url = f"https://api.trello.com/1/cards"


Use more specific names for url in each function, such as create_card_url

This has been fixed in the new version.

… asked

isiddharthsingh · 2024-12-19T08:24:57Z

All the tests have been passed.

isiddharthsingh added 7 commits November 6, 2024 20:41

trello_bot

4a55a0f

google calender sync

8fcf0f2

gmail to slack integration

47da400

.gitignore file

c837745

Remove calendar_sync.py, gmail_bot.py, and trello_bot.py from repository

dfde8e1

all files follow src/api format, integrated all bots together

d8b6697

cindyke124 reviewed Dec 16, 2024

View reviewed changes

Refactored the trello bot codes and tests file along with the changes…

6e5b023

… asked

isiddharthsingh added 2 commits December 19, 2024 03:37

remove requirements.txt

7040fe2

Update Readme

483c492

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hw3 submission #11

Hw3 submission #11

isiddharthsingh commented Dec 7, 2024

Sidhved commented Dec 16, 2024 •

edited

Loading

cindyke124 Dec 16, 2024

isiddharthsingh Dec 19, 2024

cindyke124 Dec 16, 2024

isiddharthsingh Dec 19, 2024

cindyke124 Dec 16, 2024

isiddharthsingh Dec 19, 2024

cindyke124 Dec 16, 2024

isiddharthsingh Dec 19, 2024

isiddharthsingh commented Dec 19, 2024

Hw3 submission #11

Are you sure you want to change the base?

Hw3 submission #11

Conversation

isiddharthsingh commented Dec 7, 2024

Sidhved commented Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isiddharthsingh commented Dec 19, 2024

Sidhved commented Dec 16, 2024 •

edited

Loading