Skip to content

fix/rust: fix rust ci bug#1139

Merged
theweipeng merged 3 commits intoapache:mainfrom
caicancai:rust_ci
Nov 25, 2023
Merged

fix/rust: fix rust ci bug#1139
theweipeng merged 3 commits intoapache:mainfrom
caicancai:rust_ci

Conversation

@caicancai
Copy link
Member

What do these changes do?

Related issue number

Closes #xxxx

Check code requirements

  • tests added / passed (if needed)
  • Ensure all linting tests pass, see here for how to run them

@caicancai
Copy link
Member Author

#1138

@caicancai
Copy link
Member Author

捕获 Locally tested

cc @wangweipeng2 @chaokunyang

@theweipeng
Copy link
Member

捕获 Locally tested
cc @wangweipeng2 @chaokunyang
@caicancai
As the report says, it seems like the workspace cannot include dirty files. Maybe you should create a shell script out of the workspace and run it?

@caicancai
Copy link
Member Author

caicancai commented Nov 24, 2023

捕获 Locally tested
cc @wangweipeng2 @chaokunyang
@caicancai
As the report says, it seems like the workspace cannot include dirty files? Maybe you should create a shell script out of the workspace and run it?

Yes, it can't include dirty files, we should allow dirty files, this dirty file is created and tested by myself

@caicancai
Copy link
Member Author

If I don't check for dirty files, I need to remove cargo clippy --workspace --all-features --all-targets --fix

@theweipeng
Copy link
Member

捕获 Locally tested

cc @wangweipeng2 @chaokunyang

@caicancai

As the report says, it seems like the workspace cannot include dirty files? Maybe you should create a shell script out of the workspace and run it?

Yes, it can't include dirty files, we should allow dirty files, this dirty file is created and tested by myself

May be you can create a temp test bash file out of the workspace?

@caicancai
Copy link
Member Author

Maybe now allowing dirty files is a good choice, I see a lot of projects don't actually require it

@theweipeng
Copy link
Member

Maybe now allowing dirty files is a good choice, I see a lot of projects don't actually require it

Both options are fine. as long as they are tested correctly

@caicancai
Copy link
Member Author

caicancai commented Nov 25, 2023

Maybe now allowing dirty files is a good choice, I see a lot of projects don't actually require it

Both options are fine. as long as they are tested correctly

Thank you for your review and reply. At present, ci will not pass if the test fails. You can check if there are any other problems

@caicancai
Copy link
Member Author

cc @chaokunyang

Copy link
Member

@theweipeng theweipeng left a comment

Choose a reason for hiding this comment

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

LGTM

@theweipeng theweipeng merged commit 5d42e42 into apache:main Nov 25, 2023
@caicancai caicancai deleted the rust_ci branch December 6, 2023 03:15
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