-
Notifications
You must be signed in to change notification settings - Fork 128
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
Pull release notes from keycloak-client
repository
#548
Conversation
@jonkoops I have issue that things don't work on my laptop. With building on my laptop It works as expected until this point (It starts to be really slow after the line
I am attaching the file
It is possible it is something specific to my laptop, however we should probably make sure that things are build in the reasonable time on all laptops (For me, after 10 minutes it is still stucked ) I don't have much comments to the changes. They look OK to me as long as they are working as expected. |
b29d3a4
to
4da3487
Compare
Not all builders have been ported over yet so comment out all the builders except the I am working on fixing up the other builders, as well as the GitHub release notes builder that needs to be generalized as well on the same manner. For now I am looking for feedback in the implementation of sourcing releases, and if it makes sense to build further on it. |
@jonkoops Yes, to me the approach is OK. We just need to make sure that it is not stucked on various laptops like it is currently on mine. But if you know how to fix it, the approach works for me. |
8e4acac
to
43eeb55
Compare
This is now in a place where it can be largely reviewed. The only major part that still needs to be done is converting the blogs builder and related code to pull in the generated files that are there so far. |
43eeb55
to
cd6ce91
Compare
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.
@jonkoops Nice, Thanks for the update!
I've added 2 comments inline. Besides that, I've tried the PR and have few additional comments:
- Every keycloak-client release generates blog-post with the title like
Keycloak 26.0.4 released
. But the title should be rather something likeKeycloak client 26.0.4 released
orKeycloak client libraries 26.0.4 released
? - The generated blog-post has something like this at the beginning:
To download the release go to [Keycloak downloads].
. This line should not be there for keycloak-client releases as there is not any keycloak-client download available among downloads. It should be there just for the server releases though - Link to the upgrading guide is probably incorrect in keycloak-client posts (added the comment inline about that too)
- It may be good to remove the blog-post https://github.com/keycloak/keycloak-web/blob/main/blog/2025/keycloak-client-2604.adoc in this PR? I've added this blog-post manually for the keycloak-client 26.0.4 release, but with your PR, we will have keycloak-client release blog-posts automatically generated, so we don't need that manual blog-post anymore to avoid having duplicated blog-post for keycloak-client 26.0.4 release?
cd6ce91
to
f8c7f7e
Compare
e4ccb1b
to
2483852
Compare
@jonkoops Nice! I've sent you PR jonkoops#1 where I've deleted manually written blog for keycloak-client 26.0.4 release, so we don't need duplicated blog-post for the same thing. I have one inline comment, but I've rather suggest to do it as a follow-up to merge this PR ASAP |
@@ -16,6 +17,8 @@ public class Context { | |||
private final File resourcesDir; | |||
private final File staticDir; | |||
private final File versionsDir; | |||
private final File keycloakVersionsDir; |
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.
Maybe it can be good if those stuff like keycloakVersionsDir
and keycloakClientVersionsDir
are handled dynamically, so that there is no need to update java sources once we add also node.js and keycloak.js adapter repositories? But IMO it is ok to do this as a follow-up (maybe once we are adding node.js).
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.
There is a lot in this PR that could be further generalized, but perhaps we do this when we pull in the Node.js adapter and Keycloak JS.
78a1edc
to
b993e1f
Compare
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.
See comments
b993e1f
to
7ace071
Compare
760b7e8
to
8c8eff4
Compare
@jonkoops In the last version, at the beginning of
I believe this should be removed in the keycloak-client blog-posts as those don't have any downloads (AFAIK it was done previously, but maybe re-introduced by some recent changes?) |
This was always the case, but I can add some logic to only include these for the main project. I'll add some code for it. |
Closes keycloak#547 Signed-off-by: Jon Koops <[email protected]> Signed-off-by: Marek Posolda <[email protected]> Co-authored-by: Marek Posolda <[email protected]>
8c8eff4
to
eb7c6b6
Compare
Work in progress PR to start sourcing release notes from other repositories in the organization, in this case the
keycloak-client
repository,Closes #547