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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - migrated to clap 3 #1957

Closed
wants to merge 6 commits into from
Closed

[Merged by Bors] - migrated to clap 3 #1957

wants to merge 6 commits into from

Conversation

manthanabc
Copy link
Contributor

@manthanabc manthanabc commented Mar 19, 2022 •
edited by RageKnify
Loading

This Pull Request fixes/closes #1955.

It changes the following:

  • changes structopt to clap

@codecov
Copy link

codecov bot commented Mar 19, 2022 •
edited
Loading

Codecov Report

Merging #1957 (f5e20be) into main (6498216) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1957   +/-   ##
=======================================
  Coverage   45.87%   45.87%           
=======================================
  Files         206      206           
  Lines       17102    17102           
=======================================
  Hits         7846     7846           
  Misses       9256     9256
Impacted Files Coverage Δ
boa_cli/src/main.rs 5.26% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6498216...f5e20be. Read the comment docs.

@manthanabc
Copy link
Contributor Author

i think i need a code review i am confused when to use clap::structopt and clap both seem to do the same thing

Razican
Razican reviewed Mar 19, 2022
View reviewed changes
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Thank you!! Looking very good!

boa_cli/src/main.rs Outdated
@@ -64,8 +64,8 @@ use boa_interner::Interner;
use colored::{Color, Colorize};
use rustyline::{config::Config, error::ReadlineError, EditMode, Editor};
use std::{fs::read, io, path::PathBuf};
use structopt::{clap::arg_enum, StructOpt};

//use structopt::{clap::arg_enum, StructOpt};
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is no longer needed, right?

boa_cli/src/main.rs
@@ -117,6 +117,7 @@ impl Opt {
}
}

/*
arg_enum! {
Copy link
Member

Choose a reason for hiding this comment

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

If this is no longer needed, can it be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah i think it should be removed.

@Razican
Copy link
Member

Razican commented Mar 19, 2022

I think clap::structopt might be a compatibility layer

@manthanabc
Copy link
Contributor Author

ok i removed the extra code

@manthanabc
Copy link
Contributor Author

i might try getting rid of some depreciated function as warned by clippy later tommarow

Razican
Razican approved these changes Mar 19, 2022
View reviewed changes
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Looks good from my side, but you need to run cargo fmt to format it and pass the format tests :)

@raskad
Copy link
Member

raskad commented Mar 20, 2022

bors r+

bors bot pushed a commit that referenced this pull request Mar 20, 2022
@manthanabc
migrated to clap 3 (#1957)
5fa1668 
This Pull Request fixes/closes #1955.

It changes the following:

- changes structopt to clap
@bors
Copy link

bors bot commented Mar 20, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title migrated to clap 3 [Merged by Bors] - migrated to clap 3 Mar 20, 2022
@bors bors bot closed this Mar 20, 2022
@Razican Razican added enhancement New feature or request dependencies Pull requests that update a dependency file labels Jun 1, 2022
@Razican Razican added this to the v0.15.0 milestone Jun 1, 2022
Razican pushed a commit that referenced this pull request Jun 8, 2022
@manthanabc @Razican
migrated to clap 3 (#1957)
afb4343 
This Pull Request fixes/closes #1955.

It changes the following:

- changes structopt to clap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Razican Razican Razican approved these changes

@raskad raskad raskad approved these changes

@jedel1043 jedel1043 Awaiting requested review from jedel1043

@HalidOdat HalidOdat Awaiting requested review from HalidOdat

@jasonwilliams jasonwilliams Awaiting requested review from jasonwilliams

@RageKnify RageKnify Awaiting requested review from RageKnify

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Milestone
v0.15.0
Development

Successfully merging this pull request may close these issues.

Update CLI to use Clap 3.0
3 participants
@manthanabc @Razican @raskad