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

feat: Submission-1 #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: Submission-1 #3

wants to merge 2 commits into from

Conversation

Nightshade14
Copy link

This is the re-submission of submission-1 for the mAIgic team 1.

Team Members:

Satyam Chatrola: [email protected]

Abdullah Suri: [email protected]

Eli Edme: [email protected]

Frank Zhao: [email protected]

Mihir Lovekar: [email protected]

Sidhved Warik: [email protected]

Yaxin Ke: [email protected]

@Frankbz Frankbz mentioned this pull request Oct 8, 2024
@kyotov
Copy link
Collaborator

kyotov commented Oct 9, 2024

I tentatively grade this at 95% :)

@kyotov
Copy link
Collaborator

kyotov commented Oct 30, 2024

i left some comments, but more generally, this is "too much" for HW1.
we need the minimal repository setup -- no extra code for trello, etc.

also a few follow-ups:

  • add mypy and ruff and enable all checks -- make sure they run in CI (you have some of this but make it complete)
  • add latest python version (i think we are up to 3.13 now?)
  • make the repo more "minimal"... trello and friends can come in subsequent PRs
  • remove secrets from code.

right now, tentatively this is at about 80/100 for me.
but you can keep improving it.

.circleci/config.yml Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
.python-version Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
SlackChatbot/ClientID_Secret.env Outdated Show resolved Hide resolved
@AbdullahSuri AbdullahSuri force-pushed the main branch 2 times, most recently from 60369b1 to e935376 Compare November 3, 2024 20:25
@AbdullahSuri
Copy link

We have reset the main branch to its initial commit and moved Trello and SlackChatbot enhancements to separate branches. We're working on incorporating the feedback, including adding mypy and ruff checks, updating to Python 3.13, and maintaining a minimal repository setup. Further updates will follow in dedicated PRs.

.python-version Outdated Show resolved Hide resolved
src/hello.py Outdated Show resolved Hide resolved
Add TrelloManager

feat: Implement OAuth and enhance message handling for Slack bot

fix: hw1 feedback fix

- Upgrade python from 3.11 to 3.12
- Use uv to transform the project from app to a library, exposing api only via modules (modules named with "_" to discourage direct use and import)
- Add mypy for static type checking
- Add pytest-cov for test coverage and reporting
- Add circleCI tests and coverage report
- Untracked artifact files
- The project is capable of being built with build-tools (maigic uses hatchling) and probably being published to PyPi
- Upgrade python from 3.11 to 3.12
- Use uv to transform the project from app to a library, exposing api only via modules (modules named with "_" to discourage direct use and import)
- Add mypy for static type checking
- Add pytest-cov for test coverage and reporting
- Add circleCI tests and coverage report
- Untracked artifact files
- The project is capable of being built with build-tools (maigic uses hatchling) and probably being published to PyPi
@Nightshade14
Copy link
Author

Nightshade14 commented Nov 10, 2024

HW1 fixed

The project is now updated according to the latest feedback.

CircleCI tests show up in the tests tab. Samples in case of success and failure are attached.

CircleCI all tests passed:

image

CircleCI all tests failed:

image

CircleCI all tests failed test coverage report:

image

Use this link to check exact diff. There are major changes in the file names and structure, so the diff would be less intuitive.

Check diff between submissions

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