Skip to content

Deploy JavaScript adapter to Maven repository - #23256

Closed
drichtarik wants to merge 1 commit into
keycloak:release/22.0from
drichtarik:release/22.0
Closed

Deploy JavaScript adapter to Maven repository#23256
drichtarik wants to merge 1 commit into
keycloak:release/22.0from
drichtarik:release/22.0

Conversation

@drichtarik

@drichtarik drichtarik commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

This commit adds keycloak-js-adapter.tar.gz to the list of artifacts uploaded to Maven repository

Closes #23312

@cypress

cypress Bot commented Sep 14, 2023

Copy link
Copy Markdown

Passing run #8956 ↗︎

0 527 48 0 Flakiness 0

Details:

Merge 526d0bf into ed80506...
Project: Keycloak Admin UI Commit: d7fffd2e0f ℹ️
Status: Passed Duration: 15:52 💡
Started: Sep 14, 2023 4:07 PM Ended: Sep 14, 2023 4:23 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Comment on lines +1 to +16
<assembly xmlns="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.3 http://maven.apache.org/xsd/assembly-1.1.3.xsd">
<id>dist</id>
<formats>
<format>zip</format>
</formats>
<includeBaseDirectory>false</includeBaseDirectory>
<fileSets>
<fileSet>
<directory>${project.basedir}/target</directory>
<outputDirectory>.</outputDirectory>
<includes>
<include>keycloak-*.tgz</include>
</includes>
</fileSet>
</fileSets>
</assembly> No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not needed, we should rather just attach keycloak-js-999.0.0-SNAPSHOT.tgz as it is already created by npm pack.

The format of the tgz file is explicit to support installation with npm and other package managers. For example with the new format one can do npm install keycloak-js-999.0.0-SNAPSHOT.tgz.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed, there should be no reason to re-pack this in another archive, as it is already an archive.

Comment on lines +75 to +96
<groupId>ca.szc.maven</groupId>
<artifactId>jsonpath-maven-plugin</artifactId>
<version>1.0.0</version>
<executions>
<execution>
<id>update-version</id>
<phase>process-resources</phase>
<goals>
<goal>modify</goal>
</goals>
<configuration>
<file>${basedir}/package.json</file>
<modifications>
<modification>
<expression>$.version</expression>
<value>${project.version}</value>
</modification>
</modifications>
</configuration>
</execution>
</executions>
</plugin>

@stianst stianst Sep 15, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to rather align the way the version is set in package.json with how we're doing it already in community, which is with set-version.sh script.

This also takes care of setting versions in Maven files, containers, docs, and makes sure admin client/console uses the correct versions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, this should be re-used if possible. Also because I believe the lockfile needs an update as well.

@stianst stianst left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if we really should upload keycloak-js-adapter-999.0.0-SNAPSHOT.tar.gz in community releases, but guess it doesn't really hurt so okay with doing that.

@stianst
stianst requested a review from jonkoops September 15, 2023 08:07
@stianst

stianst commented Sep 15, 2023

Copy link
Copy Markdown
Contributor

@drichtarik One question, can PNC upload non-Maven artifacts as part of the build? It doesn't really make all that much sense for us to upload a NPM package to Maven, especially not in community.

@stianst

stianst commented Sep 15, 2023

Copy link
Copy Markdown
Contributor

@drichtarik please don't link to RHBK issues in the community project. If anything needs changing in Keycloak upstream then open a GitHub Issue for it.

@stianst stianst changed the title RHBK-88 Productize client-side JS adapter Deploy JavaScript adapter to Maven repository Sep 18, 2023
@stianst stianst linked an issue Sep 18, 2023 that may be closed by this pull request
@pskopek

pskopek commented Sep 21, 2023

Copy link
Copy Markdown
Contributor

closing this PR as the issue is address with newer one #23428

@pskopek pskopek closed this Sep 21, 2023
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.

Deploy JavaScript adapter to Maven repository

4 participants