Skip to content

Refactor ota package #97

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

Merged
merged 5 commits into from
Aug 9, 2022
Merged

Refactor ota package #97

merged 5 commits into from
Aug 9, 2022

Conversation

polldo
Copy link
Contributor

@polldo polldo commented Jan 12, 2022

Motivation

The ota package used to return a WriteCloser interface VERY weird:

  • it had to be written in a one-shot fashion (and returning an io.WriteCloser that can be written only once is confusing)
  • returning an object that should be closed is confusing

interfaces should be used in the opposite way: return concrete types, takes interfaces as input. So, the consumer can then decide to close the io.Writer he decides to use (if needed).

Change description

Instead of returning a WriteCloser interface, it now returns a specific Encoder. This Encoder wraps a io.Writer that's where the encoded file will be written to.

Add tests

Additional Notes

Tested with commands ota upload and ota mass-upload

Reviewer checklist

  • PR address a single concern.
  • PR title and description are properly filled.
  • Changes will be merged in main.
  • Changes are covered by tests.
  • Logging is meaningful in case of troubleshooting.
  • History is clean, commit messages are meaningful (see CONTRIBUTING.md) and are well formatted.

@polldo polldo requested a review from rsora August 4, 2022 09:06
@polldo polldo force-pushed the polldo/ota-refactor branch from d60d2e6 to f2079e4 Compare August 4, 2022 09:10
@polldo polldo changed the base branch from main to polldo/fix-ota August 4, 2022 09:13
Copy link

@rsora rsora left a comment

Choose a reason for hiding this comment

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

Excellent work!

Base automatically changed from polldo/fix-ota to main August 9, 2022 13:42
@polldo polldo force-pushed the polldo/ota-refactor branch from e683fba to f36992b Compare August 9, 2022 13:44
@polldo polldo merged commit 74934ae into main Aug 9, 2022
@polldo polldo deleted the polldo/ota-refactor branch August 9, 2022 14:01
polldo added a commit that referenced this pull request Sep 2, 2022
Return a specific 'Econder' struct instead of a 'WriteCloser' interface. The 'Encoder' wraps a 'io.Writer' that's where the encoded file will be written to. In this way the consumer can decide when and how to open/close the writer.
Add tests.
polldo added a commit that referenced this pull request Sep 2, 2022
Return a specific 'Econder' struct instead of a 'WriteCloser' interface. The 'Encoder' wraps a 'io.Writer' that's where the encoded file will be written to. In this way the consumer can decide when and how to open/close the writer.
Add tests.
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.

2 participants