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

cargo: add support for GitLab #29

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

yamader
Copy link

@yamader yamader commented Aug 19, 2024

Added GitLab support to cargo.py.
Additional tests have not yet been written, but the tests that already exist have passed.

As for demand, the following project uses (self-hosted) GitLab.
https://github.com/pop-os/xdg-desktop-portal-cosmic/blob/41c1e7c/Cargo.lock#L4633

Signed-off-by: Daichi Yamamoto <dev@dyama.net>
Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

I'm sorry for replying this late.

pycargoebuild/cargo.py Outdated Show resolved Hide resolved
pycargoebuild/cargo.py Outdated Show resolved Hide resolved
pycargoebuild/cargo.py Outdated Show resolved Hide resolved
pycargoebuild/cargo.py Outdated Show resolved Hide resolved
pycargoebuild/cargo.py Show resolved Hide resolved
Signed-off-by: Daichi Yamamoto <dev@dyama.net>
@yamader
Copy link
Author

yamader commented Nov 4, 2024

Thank you for reviewing my poor code.
I have fixed the issues you pointed out.

The py3.9 test fails because I used a match statement, but py39 has been dropped in the latest pycargoebuild ebuild.
Please let me know if it would be better to rewrite it to be compatible.

Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

I think py3.10+ is fine but please update GHA and tox.ini.

pycargoebuild/cargo.py Outdated Show resolved Hide resolved
pycargoebuild/cargo.py Outdated Show resolved Hide resolved
pycargoebuild/cargo.py Outdated Show resolved Hide resolved
@mgorny
Copy link
Member

mgorny commented Nov 4, 2024

…and pyproject.toml.

Signed-off-by: Daichi Yamamoto <dev@dyama.net>
Signed-off-by: Daichi Yamamoto <dev@dyama.net>
@yamader
Copy link
Author

yamader commented Nov 4, 2024

fixed.

@mgorny
Copy link
Member

mgorny commented Nov 4, 2024

Thanks. But please extend test_cargo to cover the new possible URLs. You can also extend integration_tests, but I can do that for you if you don't want to figure them out.

Signed-off-by: Daichi Yamamoto <dev@dyama.net>
@yamader
Copy link
Author

yamader commented Nov 5, 2024

I added a test for this PR.

It was difficult to find a good example of using gitlab.com, but there are many projects using self-hosted GitLab, such as pop-os/cosmic-* and YaLTeR/niri, mainly for desktop systems.

However, complete support for self-hosted GitLab that can be built directly by running pycargoebuild cannot be achieved without updating cargo.eclass, but I would like to work on that separately.

@mgorny
Copy link
Member

mgorny commented Nov 5, 2024

Actually, adding a test to test_cargo.py is more important than integration tests.

Signed-off-by: Daichi Yamamoto <dev@dyama.net>
@yamader
Copy link
Author

yamader commented Nov 7, 2024

Added. Please forgive my stupidity.

@mgorny
Copy link
Member

mgorny commented Nov 7, 2024

Thank you. I think I'll test it a little more, and merge.

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