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

osfv integration #30

Merged
merged 7 commits into from
Jun 11, 2024
Merged

osfv integration #30

merged 7 commits into from
Jun 11, 2024

Conversation

PLangowski
Copy link
Contributor

No description provided.

@PLangowski PLangowski force-pushed the osfv-integration branch 2 times, most recently from cc6f8a0 to 3316169 Compare April 17, 2024 08:22
@PLangowski PLangowski force-pushed the osfv-integration branch 2 times, most recently from 623e8ff to 226b006 Compare April 18, 2024 09:30
@PLangowski PLangowski changed the title [WIP]: osfv integration osfv integration Apr 18, 2024
@PLangowski PLangowski requested a review from macpijan April 18, 2024 14:16
Copy link
Contributor

@macpijan macpijan left a comment

Choose a reason for hiding this comment

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

Looks rather OK. Left one idea on decoupling the RTE module from SnipeIT module, which I think we should consider.

Other than that, I'd ask you to clean up the commits as well.
The merge commit should also not be there.

@PLangowski PLangowski force-pushed the osfv-integration branch 6 times, most recently from 2cb1983 to f9c6bd5 Compare April 23, 2024 07:03
@PLangowski PLangowski requested a review from macpijan April 23, 2024 07:30
Copy link
Contributor

@macpijan macpijan left a comment

Choose a reason for hiding this comment

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

@PLangowski Code looks OK, but we still need to:

  • cleanup commits - we specifically do not want to merge commits like wip, or refactor, if possible; we should split the whole changes into a few logical commits
  • pre-commit fails, possibly you do not have it installed locally in this repo?

@PLangowski PLangowski force-pushed the osfv-integration branch 3 times, most recently from 0eae6ba to 5c372ec Compare April 25, 2024 08:10
@PLangowski
Copy link
Contributor Author

@macpijan I cleaned up the commits.

@PLangowski PLangowski requested a review from macpijan April 25, 2024 08:11
@macpijan macpijan mentioned this pull request Jun 11, 2024
Signed-off-by: Maciej Pijanowski <[email protected]>
Signed-off-by: Maciej Pijanowski <[email protected]>
@macpijan macpijan merged commit 5747856 into main Jun 11, 2024
1 check passed
@macpijan macpijan deleted the osfv-integration branch June 11, 2024 18:45
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.

Use OSFV python lib to implement low-level interfaces, like flashing
2 participants