minishift / minishift-addons

A repository for the community to exchange Minishift add-ons
Apache License 2.0
71 stars 86 forks source link

fabric8-launcher: make Launcher Booster Catalog customizable #148

Closed vorburger closed 2 years ago

vorburger commented 6 years ago

Useful e.g. for @edewit's https://github.com/edewit/minecraft-sponge-plugin-s2i/issues/4, but also for anyone else who wants to use the fabric8-launcher addon but with a custom Booster Catalog.

There is a comment at the top of https://raw.githubusercontent.com/fabric8-launcher/launcher-documentation/master/docs/topics/proc_installing-launcher-tool-addon.adoc for something similar? Perhaps this PR should be pointed out to someone working on that - @tradej @inoxx03 @rhoads-zach ? BTW: I don't understand how that documentation can possibly be a "parametrized module", given that parameter-custom-catalog doesn't actually appear in that doc?

It's Not Great at all that I had to copy/paste and repeat the default values for the ..REPOSITORY and ..REF from the original launcher-template.yaml here - anyone knows if there is a better way?

I haven't actually tested this, yet.

centos-ci commented 6 years ago

Can one of the admins verify this patch?

praveenkumar commented 6 years ago

@vorburger Can someone from your team or you test this change because we are not so expert about what to test here and how it should behave.

vorburger commented 6 years ago

@praveenkumar I'll test it, but do you have any idea / know anyone who could help re. a better way to avoid the default values for the ..REPOSITORY and ..REF from the original launcher-template.yaml here?

praveenkumar commented 6 years ago

@vorburger I am not sure, might be good to ask on launcher github issue tracker?

vorburger commented 6 years ago

@praveenkumar but it isn't really a Launcher specific issue... this is more of a minishift-addon thing: So I want users to have the option to override those two things, but they are options which already have default values, in the YAML template. So if a user does not specify those 2 options, those should be used. But I have copy/pasted those from the YAML template into the minishift addon, and in an ideal world that should be avoided I guess. Do you see what I mean?

praveenkumar commented 6 years ago

@vorburger let me know if I understand correctly, as of now you put those value as part of Default-Vars but you also want this value can be over ride by user then also put those variable as part of Required-Vars so that user can override using like below.

$ minishift addon apply -a "LAUNCHER_BOOSTER_CATALOG_REPOSITORY=foo.com" -a "LAUNCHER_BOOSTER_CATALOG_REF=bar"
vorburger commented 2 years ago

Hello @praveenkumar I'm doing a year end clean up of my personal https://github.com/pulls and wanted to ask if you would still consider merging this very old Pull Request if I rebased it, or if we should forget about and close this? (This is a bulk message I'm posting to many issues to gauge what is still revelant.)

gbraad commented 2 years ago

We do not support minishift anymore. The tool was made EOL at the end of August.

No member of the team has time to spend on testing or rebasing. If you do this, perhaps it can be merged.

On Sat, Dec 4, 2021, 18:00 Michael Vorburger ⛑️ @.***> wrote:

Hello @praveenkumar https://github.com/praveenkumar I'm doing a year end clean up of my personal https://github.com/pulls and wanted to ask if you would still consider merging this very old Pull Request if I rebased it, or if we should forget about and close this? (This is a bulk message I'm posting to many issues to gauge what is still revelant.)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/minishift/minishift-addons/pull/148#issuecomment-986001943, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAOZXXJOOR4QJIYTYACV3UPHRFRANCNFSM4FPEZSAQ .