+
Skip to content

Copyied source files directly in the keycloak-client #14

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 25, 2024
Merged

Copyied source files directly in the keycloak-client #14

merged 2 commits into from
Jul 25, 2024

Conversation

mposolda
Copy link
Contributor

PR, which adds Java sources directly as part of this repository to not have any dependency on the artifacts from keycloak repository during the build.

The PR is divided into 2 commits.

Commit 1:

  • Trigger the maven-dependency-plugin run just when the maven is executed with the sync profile
  • Adding the bash script sync-keycloak-sources.sh, which triggers the maven with the sync profile from the above and then it adds the Java classes directly into the codebase of keycloak-client

Commit 2:

  • This is just a result of running .github/scripts/sync-keycloak-sources.sh , which means Java files and few resources added into the workspace

The further mvn clean install command should be able to use the sources directly from the modules without need to run maven-dependency-plugin and hence hopefully not having any dependencies on keycloak artifacts

mposolda added 2 commits July 24, 2024 20:47
@mposolda
Adding profile 'sync'. Adding script sync-keycloak-sources.sh to trig…
c1fdd93 
…ger syncing Keycloak sources

Signed-off-by: mposolda <mposolda@gmail.com>
@mposolda
Copied Java classes from keycloak repository
4c0dfd8 
Signed-off-by: mposolda <mposolda@gmail.com>
@mposolda mposolda changed the title Copyinf files 1 Copyied source files directly in the keycloak-client Jul 24, 2024
@mposolda mposolda requested review from pskopek, jonkoops and stianst July 24, 2024 19:12
@mposolda
Copy link
Contributor Author

mposolda commented Jul 25, 2024 •
edited
Loading

@jonkoops @pskopek @stianst The PR does not preserve the github history of the Java files, which are copied from the keycloak repository. Few things about it:

  • The "authoritative version" of the Java files is available in keycloak repository where it can be seen who did changes to particular files etc.

  • Attempt to preserve history will make history quite messy in the keycloak-client repository IMO. If we sync the files by the bash script with maven-dependency-plugin and we always send the PR, we will have cleaner history in the keycloak-client and we will see, who did the last sync with keycloak repository and when it was synced etc. So ideally, we will have just history like:

* abc Sync with Keycloak after Keycloak 26.0.2 release
* def Sync with Keycloak after 26.0.1 Keycloak release
*ghi Sync with Keycloak after Organization endpoints added

which IMO has much more added value rather than messy GH history with all the commit history preserved of the original Java files.

  • It will require different approach than current bash script with maven-dependency-plugin though

jonkoops
jonkoops approved these changes Jul 25, 2024
View reviewed changes
Copy link
Contributor

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

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

LGTM

pskopek
pskopek approved these changes Jul 25, 2024
View reviewed changes
Copy link
Collaborator

@pskopek pskopek left a comment

Choose a reason for hiding this comment

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

Looks good to me!
No need for history as source of truth is keycloak/keycloak.git.

@mposolda mposolda merged commit 10d37a7 into keycloak:main Jul 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pskopek pskopek pskopek approved these changes

@jonkoops jonkoops jonkoops approved these changes

@stianst stianst Awaiting requested review from stianst

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mposolda @pskopek @jonkoops
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载