Fix/issue 6 - #208
Conversation
|
@EHEGUY is attempting to deploy a commit to the komalsony234-1530's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@komalharshita done ! |
komalharshita
left a comment
There was a problem hiding this comment.
Thanks for the contribution — the scoring weights refactor itself is clean and the added regression test is a good addition.
However, this PR currently mixes two unrelated changes:
- Refactoring the recommendation scoring weights into
SCORING_WEIGHTS - Adding category badge UI changes to the project detail page
The linked issue (#6) only covers the scoring refactor. The badge system and related CSS/template updates belong to a separate feature and should be submitted in an independent PR.
Please remove the unrelated badge/UI commits from this PR (or open a separate PR for them) so the contribution stays properly scoped to the assigned issue.
Once the PR contains only the scoring refactor + associated tests, it will be much easier to review and merge.
|
Hi @komalharshita Thank you for the detailed feedback really appreciate you taking the time to explain exactly what was wrong. I've removed the unrelated badge/UI commits from this branch. The PR now contains only the All 28 tests pass locally. Ready for re-review whenever you get a chance! |
|
@komalharshita the merge conflict has been solved can u review it ! |
Summary [required]
This PR refactors the recommendation engine by grouping the four individual scoring weight constants into a single
SCORING_WEIGHTSdictionary. This makes the configuration cleaner and easier to manage.score_single_project()has been updated to use these dictionary values, and a test has been added to ensure the dictionary maintains the correct structure.Related Issue [required]
Closes #6
Type of Change [required]
data/projects.jsonWhat Was Changed [required]
utils/recommender.pySCORING_WEIGHTSdictionary. Updated references inscore_single_project().tests/test_basic.pytest_scoring_weights_has_all_keysto verify the new dictionary contains the required keys.How to Test This PR [required]
git checkout fix/issue-6pip install -r requirements.txtpython app.pypython tests/test_basic.pyExpected test output:
Test Results [required]
Self-Review Checklist [required]
feat/,fix/,docs/,data/,style/,test/python tests/test_basic.pyand all 28 tests passflake8 .locally and there are no errorsprint()orconsole.log()debug statementsNotes for Reviewer
Removed the unrelated badge/UI commits from the previous push this PR now contains only the scoring refactor and associated test.