-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[AlpinePackages] New apline packages bridge #4781
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
base: master
Are you sure you want to change the base?
Conversation
Pull request artifacts
last change: Sunday 2026-01-25 10:54:12 |
|
With my other pull request, I see that I will want to make some changes here also. |
Mynacol
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bridge is rather simple and self-explanatory. I still couldn't test locally (urlencode).
|
Made a lot of changes. probably requires a re-review. Any comment is welcome. |
Mynacol
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for the overhaul. I only have remaining minor points.
| 'name' => 'Package Name', | ||
| 'required' => true, | ||
| 'exampleValue' => 'curl', | ||
| 'title' => 'Name of the package. Use * and ? as wildcards. Example: curl, curl-* or curl-???' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use a consistent package for all three examples you give? E.g.
| 'title' => 'Name of the package. Use * and ? as wildcards. Example: curl, curl-* or curl-???' | |
| 'title' => 'Name of the package. Use * and ? as wildcards. Example: curl-dev, curl-* or curl-???' |
| 'name' => 'Package branch', | ||
| 'required' => true, | ||
| 'exampleValue' => 'v3.23', | ||
| 'title' => 'Name of the branch. Like: edge, v3.23, v3.22, etc.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'title' => 'Name of the branch. Like: edge, v3.23, v3.22, etc.' | |
| 'title' => 'Name of the branch. For example: edge, v3.23, v3.22, etc.' |
| 'Testing' => 'testing' | ||
| ], | ||
| 'defaultValue' => 'all', | ||
| 'title' => 'Name of the repository.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd delete that if it doesn't add any detail to the name above.
| } | ||
| } | ||
|
|
||
| public function getName() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output is quite a mouthful. Can we develop a more compact representation for the feed name?
E.g. curl (branch edge, repo main, arch x86_64) - Alpine packages.
Also, we might skip a part if it is empty or "all."
| } | ||
| if ($repositoryName) { | ||
| $repositoryName = strtolower($repositoryName); | ||
| if ($repositoryName === 'all') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This handles "all" different from an empty specification. Please handle them the same (as in getUri below).
This bridge will check if an Alpine package is update. Specify: branch, repository, architecture and package.
It will only specify the new version.