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

refactor: clap upgrade to v4 #109

Merged
merged 3 commits into from
Aug 13, 2024
Merged

Conversation

CHOUMnote
Copy link
Contributor

Previously, we were using version 3 of Clap, so we upgraded to version 4. However, this caused compile errors due to the version upgrade, leading us to remove a lot of legacy code and refactor the project.

Note) I tested the debug option as thoroughly as possible, but I was unable to test the dump option because I didn't know how to use it.

Copy link

cla-assistant bot commented Aug 10, 2024

CLA assistant check
All committers have signed the CLA.

@CHOUMnote CHOUMnote changed the title 'clap' upgrade(v3 to v4) and refactoring clap upgrade(v3 to v4) and refactoring Aug 10, 2024
@jopemachine jopemachine linked an issue Aug 12, 2024 that may be closed by this pull request
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@jopemachine jopemachine left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!
Please check the review

@CHOUMnote
Copy link
Contributor Author

@jopemachine I have applied the code. Please review it.

raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
raftify/src/cli/mod.rs Outdated Show resolved Hide resolved
@jopemachine jopemachine changed the title clap upgrade(v3 to v4) and refactoring refactor: clap upgrade to v4 Aug 13, 2024
@CHOUMnote
Copy link
Contributor Author

i have a question. do you recommend not to write comments as much as possible?

@jopemachine
Copy link
Member

i have a question. do you recommend not to write comments as much as possible?

No, I'm not saying don't comment; rather, the code should be self-explanatory.

Comments should explain things that cannot be explained by the code itself, not repeat what the code already expresses.

I think this code is already sufficiently self-explanatory, so additional comments are not necessary.

@jopemachine jopemachine merged commit 5a6fe2f into lablup:main Aug 13, 2024
1 check passed
@jopemachine
Copy link
Member

jopemachine commented Aug 13, 2024

@CHOUMnote Thank you for your contribution! ❤️

@CHOUMnote CHOUMnote deleted the kjh/feature/up_clap branch August 15, 2024 05:43
@CHOUMnote CHOUMnote restored the kjh/feature/up_clap branch August 15, 2024 05:43
@CHOUMnote CHOUMnote deleted the kjh/feature/up_clap branch August 15, 2024 05:43
@jopemachine jopemachine added refactoring Rewrite something in better way while keeping the same functionality ossca-24 OSS Contribution Academy mentee's contributions. labels Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli ossca-24 OSS Contribution Academy mentee's contributions. refactoring Rewrite something in better way while keeping the same functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade clap to v4
2 participants