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

sea as an alternative bin name to sea-orm-cli #558

Merged
merged 3 commits into from
Mar 26, 2022

Conversation

ZhangHanDong
Copy link
Contributor

@ZhangHanDong ZhangHanDong commented Feb 26, 2022

I feel that the command sea-orm-cli is too long and inconvenient to use, so it should be more convenient to simplify it to sea now.

What do you think?

Changes

The sea command is now an option, and a new bin file has been added to achieve this.

The only downside: bin/sea.rs is a duplicate of bin/main.rs, but it has now been modified to have only one main function, so if there are changes, they are simply copied to sea.rs. It won't be too much trouble to maintain.

Now cargo run still defaults to sea-orm-cli

Install and Usage:

> cargo install sea-orm-cli 
> sea-orm-cli help

Or:

> cargo install --bin sea
> sea help

@tyt2y3
Copy link
Member

tyt2y3 commented Feb 26, 2022

I think this is a reasonable thought. sea-orm-cli is more canonical, but sea is a perfectly fine shorthand. Is it possible to make it an alias?

@ZhangHanDong
Copy link
Contributor Author

ZhangHanDong commented Feb 26, 2022

I think this is a reasonable thought. sea-orm-cli is more canonical, but sea is a perfectly fine shorthand. Is it possible to make it an alias?

It doesn't seem to be possible to configure aliases in the toml file, but you can configure multiple bins like this, which will generate two main target files:

[[bin]]
name = "sea-orm-cli"
path = "src/main.rs"

[[bin]]
name = "sea"
path = "src/main.rs"

it's ok?

@tyt2y3
Copy link
Member

tyt2y3 commented Feb 26, 2022

That's interesting! Certainly better than a breaking name change. I also think we need to set the default binary, other cargo run will confuse

@ZhangHanDong
Copy link
Contributor Author

ZhangHanDong commented Feb 26, 2022

That's interesting! Certainly better than a breaking name change. I also think we need to set the default binary, other cargo run will confuse

This has the minor drawback of reporting a compile warning at compile time:

warning: /Projects/sea-orm/sea-orm-cli/Cargo.toml: file found to be present in multiple build targets

@ZhangHanDong
Copy link
Contributor Author

@tyt2y3

The sea command is now an option, and a new bin file has been added to achieve this.

The only downside: bin/sea.rs is a duplicate of bin/main.rs, but it has now been modified to have only one main function, so if there are changes, they are simply copied to sea.rs. It won't be too much trouble to maintain.

Now cargo run still defaults to sea-orm-cli

Install and Usage:

> cargo install sea-orm-cli 
> sea-orm-cli help

Or:

> cargo install --bin sea
> sea help

@ZhangHanDong
Copy link
Contributor Author

@tyt2y3 what do you think?

@tyt2y3
Copy link
Member

tyt2y3 commented Feb 27, 2022

Awesome

@ZhangHanDong
Copy link
Contributor Author

@tyt2y3 Fixed CI compile error

@ZhangHanDong
Copy link
Contributor Author

@tyt2y3 need approve workflow

@ZhangHanDong
Copy link
Contributor Author

@tyt2y3

Can't it be merged yet? Is there anything that needs to be changed?

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

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

LGTM

@ZhangHanDong
Copy link
Contributor Author

ZhangHanDong commented Mar 3, 2022

@tyt2y3

P.S Can't seaorm automatically generate migration files now? Except for the sea migrate init.

@billy1624
Copy link
Member

@tyt2y3

P.S Can't seaorm automatically generate migration files now? Except for the sea migrate init.

Not supported at the moment. I think you want something like sea migrate new name_of_new_migration_file which create a new migration file?

@ZhangHanDong
Copy link
Contributor Author

Yes. Like the rails migrate command

@tyt2y3 tyt2y3 changed the title Simplify bin name from sea-orm-cli to sea sea as an alternative bin name to sea-orm-cli Mar 26, 2022
@tyt2y3 tyt2y3 merged commit fe1877a into SeaQL:master Mar 26, 2022
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.

3 participants