Skip to content

Add support for similar waste types with different API IDs#1267

Merged
frenck merged 2 commits intofrenck:mainfrom
EyeDevelop:feature/new-waste-type-56
Dec 7, 2024
Merged

Add support for similar waste types with different API IDs#1267
frenck merged 2 commits intofrenck:mainfrom
EyeDevelop:feature/new-waste-type-56

Conversation

@EyeDevelop
Copy link
Copy Markdown
Contributor

Proposed Changes

This PR adds a fall-through method to the WasteType enum. The Twente Milieu API might return a waste type that is almost the same as one already defined in the enum.

Hengelo now has pick-up points for packaging and paper for high-density living. For the packaging, they created a new waste type. However, it is still packaging. This change now maps the 'odd' waste type returned from the API to the packages value in the enum.

Please let me know what you think!

Related Issues

#1265

When attempting to run the `updateContentCommand` while building the development container, it returned an error with git as cause. The log stated that the directory was not marked safe, so now it does that before the `updateContentCommand`.
@frenck frenck added the enhancement Enhancement of the code, not introducing new features. label Dec 6, 2024
Copy link
Copy Markdown
Owner

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Hi there @EyeDevelop 👋

CI is failing, could you please take a look?

../Frenck

@frenck frenck marked this pull request as draft December 6, 2024 22:16
@EyeDevelop
Copy link
Copy Markdown
Contributor Author

Hi @frenck, I (should) have resolved all the issues with CI. Can you take a look?

I like the idea of having strict coding guidelines enforced with these typing rules and test coverage. However, if I could make a suggestion it would be to loosen them in some cases. For the _missing function I added to the IntEnum, mypy made a note that value should be of type object. However, in practice this can never be anything other than int unless the _missing_ function is directly called. You can see in the test I added that I have to call the method directly to comply with the 100% coverage. It is my opinion that these are not really useful additions, but I leave this up to you.

@EyeDevelop EyeDevelop marked this pull request as ready for review December 7, 2024 12:33
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7c783e6) to head (95481e9).
Report is 362 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1267   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           95       100    +5     
  Branches        15        12    -3     
=========================================
+ Hits            95       100    +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Owner

@frenck frenck left a comment

Choose a reason for hiding this comment

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

However, if I could make a suggestion it would be to loosen them in some cases.

Sorry... I strongly disagree very much. Don't know what else to say.

Left a few review comments and the CI is also failing (which is odd? As ruff is run in pre-commit... not sure how you manage to make a commit that passes it).

../Frenck

@EyeDevelop
Copy link
Copy Markdown
Contributor Author

Processed your review comments. Let me know if I should drop the first commit with the changes to the dev container file.

Copy link
Copy Markdown
Owner

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Thanks, @EyeDevelop 👍

../Frenck

@frenck frenck merged commit 90c0013 into frenck:main Dec 7, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement Enhancement of the code, not introducing new features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants