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

Also include stream information in CLI project creation #34034

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Jun 14, 2023

This has been done a long while ago for standard apps but I missed to update the CLI somehow.

This has been done a long while ago for standard apps but I missed to
update the CLI somehow.
@github-actions
Copy link

github-actions bot commented Jun 14, 2023

🙈 The PR is closed and the preview is expired.

@gsmet
Copy link
Member Author

gsmet commented Jun 14, 2023

@MichalMaler pinging you as I'm not sure if the downstream doc overrides create-app-group-id. If so, it will have to override create-cli-group-id too after this PR is merged.

@MichalMaler
Copy link
Contributor

@MichalMaler pinging you as I'm not sure if the downstream doc overrides create-app-group-id. If so, it will have to override create-cli-group-id too after this PR is merged.

Hello. I think downstream not override anything ATM. We have this attribute specified in our attributes.adoc:
:create-app-group-id: org.acme

I will ask @inoxx03 for confirmation. I guess he will have more info about this too :)
Stefan, please, do we have any plan about this attribute being defined downstream with a specific value?

@inoxx03
Copy link

inoxx03 commented Jun 19, 2023

@gsmet I've run a search of our docs set and can confirm that a the moment we do not override the create-app-group-id in the downstream docs.

Form what i know, we'll need to override the platform {quarkus-platform-groupid} when we downstream the automated upgrade guide, but we have been leaving the org.acme example project name in the downstream docs so far.

From a long term perspective, I wouldn't exclude the possibility that we might want to override it at some point, in which case we can do 2 things downstream:

  • override the create-cli-group-id attribute instead of create-app-group-id (Works if we are reusing the complete upstream attribute file downstream, albeit the attribute name would be slightly counterintuitive in the context of downstream docs)
  • define our own value for the create-app-group-id attribute. (this would only work if we maintain our own set of downstream attributes in a separate attributes.adoc file which would only live downstream)

Either way I don't think that this change is going to break our docs. @MichalMaler please feel free to follow-up if you know that this change carries a risk of breaking anything downstream, although I don't think it does at the moment.

Why exactly was the attribute name change necessary?

@gsmet gsmet merged commit e865c90 into quarkusio:main Jun 20, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 20, 2023
@gsmet
Copy link
Member Author

gsmet commented Jun 20, 2023

Thanks, I will merge it then.

@maxandersen
Copy link
Member

just a quick comment - the whole intent on how we made quarkus have notion of platforms and streams is to avoid having to document different behaviors. it should be sufficient to just enable the product registry first and it should use the recommended defaults from that registry.

@maxandersen
Copy link
Member

so if some productized docs had to make changes like this we would be making differences that shouldn't be necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants