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

Minor improvements #9

Merged
merged 8 commits into from
Aug 14, 2024
Merged

Minor improvements #9

merged 8 commits into from
Aug 14, 2024

Conversation

isinyaaa
Copy link
Contributor

@isinyaaa isinyaaa commented Aug 12, 2024

Some improvement suggestions from a quick look around. Tested with existing tests, but I still haven't been able to run e2e.

Copy link
Member

@tarilabs tarilabs left a comment

Choose a reason for hiding this comment

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

thanks @isinyaaa much appreciated !

I'm not a big fan of reformattings, but I'd take it.

Can you kindly clarify/help me better understand the followings:

About the E2E test failure, see below:

omlmd/cli.py Outdated Show resolved Hide resolved
@isinyaaa
Copy link
Contributor Author

thanks @isinyaaa much appreciated !

I'm not a big fan of reformattings, but I'd take it.

I can disable my auto-formatter if you prefer, but having some sort of style guidelines would be helpful for contributions.

Can you kindly clarify/help me better understand the followings:

when you declare from __future__ import annotations it enables forward compatibility for type annotations.

You don't need it because oras is already opening that file in the required mode (w+b, not w).

@tarilabs
Copy link
Member

thanks for the clarification @isinyaaa , can you please reword your commits to sign-off for DCO, ...or just squash them if easier to add the required DCO

Signed-off-by: Isabella do Amaral <[email protected]>
Signed-off-by: Isabella do Amaral <[email protected]>
Signed-off-by: Isabella do Amaral <[email protected]>
Signed-off-by: Isabella do Amaral <[email protected]>
Signed-off-by: Isabella do Amaral <[email protected]>
Signed-off-by: Isabella do Amaral <[email protected]>
@tarilabs
Copy link
Member

many thanks again, proceeding to merge

@tarilabs tarilabs merged commit c5c2797 into containers:main Aug 14, 2024
11 checks passed
@isinyaaa isinyaaa deleted the push-yrysvwywnkyx branch August 14, 2024 12:43
@isinyaaa isinyaaa restored the push-yrysvwywnkyx branch August 14, 2024 12:43
@isinyaaa isinyaaa deleted the push-yrysvwywnkyx branch August 14, 2024 12:43
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