Skip to content

curl: support VLAN Priority: --vlan-priority #13907

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

Closed
wants to merge 1 commit into from

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Jun 7, 2024

Add --vlan-priority option to the command line tool for setting VLAN priority.

@github-actions github-actions bot added the CI Continuous Integration label Jun 7, 2024
@orgads orgads force-pushed the vlan-priority branch 2 times, most recently from 9eae92e to cf37166 Compare June 7, 2024 11:04
@orgads
Copy link
Contributor Author

orgads commented Jun 7, 2024

What's the hyper check? It failed and there are no logs whatsoever.

@icing
Copy link
Contributor

icing commented Jun 7, 2024

What's the hyper check? It failed and there are no logs whatsoever.

Looks like github had a problem. When the CI workflow is done, try restarting the failed jobs.

@orgads orgads force-pushed the vlan-priority branch 5 times, most recently from b2d4089 to 6aa6208 Compare June 11, 2024 05:42
@orgads
Copy link
Contributor Author

orgads commented Jun 11, 2024

These flaky tests are really frustrating...

@orgads
Copy link
Contributor Author

orgads commented Jun 11, 2024

I moved the "unsupported" handling to the callback (in a separate commit). If you like it, you can leave it or squash. Otherwise, I'll discard this commit (probably tomorrow night).

@bagder
Copy link
Member

bagder commented Jun 11, 2024

I moved the "unsupported" handling to the callback (in a separate commit). If you like it, you can leave it or squash. Otherwise, I'll discard this commit (probably tomorrow night).

I prefer to detect the problem in the earlier stage rather than delaying and erroring out after having done more processing, when it is possible. For that reason prefer the previous take a little more.

Also, I think we should use errorf() instead of warnf() for this since we bail out and return an error for this case.

Add --vlan-priority option to the command line tool for setting VLAN
priority.
@orgads
Copy link
Contributor Author

orgads commented Jun 11, 2024

I moved the "unsupported" handling to the callback (in a separate commit). If you like it, you can leave it or squash. Otherwise, I'll discard this commit (probably tomorrow night).

I prefer to detect the problem in the earlier stage rather than delaying and erroring out after having done more processing, when it is possible. For that reason prefer the previous take a little more.

Also, I think we should use errorf() instead of warnf() for this since we bail out and return an error for this case.

Done. Better now?

@bagder bagder closed this in 54fe8c4 Jun 11, 2024
@bagder
Copy link
Member

bagder commented Jun 11, 2024

Thanks!

@orgads orgads deleted the vlan-priority branch June 12, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration cmdline tool
Development

Successfully merging this pull request may close these issues.

4 participants