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

regeneration to be based on latest generation, and for lyoVersion="5.0.1-SNAPSHOT" #281

Merged
merged 3 commits into from
Oct 2, 2022

Conversation

jadelkhoury
Copy link
Contributor

@jadelkhoury jadelkhoury commented Oct 2, 2022

Description

regeneration to be based on latest generation, and for lyoVersion="5.0.1-SNAPSHOT"

Checklist

  • This PR adds an entry to the CHANGELOG. See https://keepachangelog.com/en/1.0.0/ for instructions. Minor edits are exempt.
  • This PR was tested on at least one Lyo OSLC server or adds unit/integration tests.
  • This PR does NOT break the API

@jadelkhoury
Copy link
Contributor Author

@berezovskyi Can you please check the changes to the pom.xml file?
domains/oslc-domains/pom.xml

What is committed is exactly what is generated! What do we need to keep manually? What userblocks can we add to avoid this in future?

Copy link
Contributor

@berezovskyi berezovskyi left a comment

Choose a reason for hiding this comment

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

I don't think any of the POM file changes should be made (maybe adding the dependencies_final user block is good for consistency).

@@ -20,12 +20,34 @@
<packaging>jar</packaging>
<name>Lyo :: Domains</name>
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these properties are already set by the parent. It will be hard to debug when we update a parent property and nothing changes because these properties override the parent ones.

<!-- TODO: Add additional repositories here to avoid them be overridden upon future re-generation -->
<!-- End of user code
-->
<repository>
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, defined in the parent

@@ -38,23 +60,35 @@
<!-- End of user code
-->
<!-- General dependencies -->
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

the libraries shall NEVER specify a logging backend, only the end user app does this (simple or log4j etc.)

<scope>provided</scope>
<groupId>javax.servlet</groupId>
<artifactId>jstl</artifactId>
<version>1.2</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

We only want to depend on the API here, not on the implementation. Adaptors can depend on implementations if they wish.

<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<version>3.1.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

The version is <dependencyManagement> versioned uniformly for all Lyo projects.

<!-- Lyo dependencies -->
<dependency>
<groupId>org.eclipse.lyo.oslc4j.core</groupId>
<artifactId>oslc4j-core</artifactId>
<version>${version.lyo}</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, the version is <dependencyManagement> versioned uniformly for all Lyo projects.

@berezovskyi
Copy link
Contributor

I also think we can check This PR does NOT break the API because constants are only added, not removed.

@jadelkhoury
Copy link
Contributor Author

I reverted the pom changes. Merging.

@jadelkhoury jadelkhoury merged commit cd4380e into master Oct 2, 2022
@jadelkhoury jadelkhoury deleted the pacakges branch October 2, 2022 14:17
@berezovskyi
Copy link
Contributor

Jenkins complains:

An error has occurred in Javadoc report generation: 
[ERROR] Exit code: 1 - /home/jenkins/agent/workspace/Lyo_multibranch_master/domains/oslc-domains/src/main/java/org/eclipse/lyo/oslc4j/core/model/package-info.java:26: warning: a package-info.java file has already been seen for package org.eclipse.lyo.oslc4j.core.model
[ERROR] package org.eclipse.lyo.oslc4j.core.model;
[ERROR]                                    ^
[ERROR] /home/jenkins/agent/workspace/Lyo_multibranch_master/domains/oslc-domains/src/main/java/org/eclipse/lyo/oslc4j/core/model/package-info.java:18: error: package org.eclipse.lyo.oslc4j.core.model has already been annotated
[ERROR] @OslcSchema ({
[ERROR] ^
[ERROR] 
[ERROR] Command line was: /opt/tools/java/temurin/jdk-11/11.0.13+8/bin/javadoc @options @packages
[ERROR] 

@jadelkhoury
Copy link
Contributor Author

Jenkins complains:

An error has occurred in Javadoc report generation: 
[ERROR] Exit code: 1 - /home/jenkins/agent/workspace/Lyo_multibranch_master/domains/oslc-domains/src/main/java/org/eclipse/lyo/oslc4j/core/model/package-info.java:26: warning: a package-info.java file has already been seen for package org.eclipse.lyo.oslc4j.core.model
[ERROR] package org.eclipse.lyo.oslc4j.core.model;

What do you think about removing Discussion from the model? It's the only resource from the Core model we have.

https://github.com/eclipse/lyo/tree/master/domains/oslc-domains/src/main/java/org/eclipse/lyo/oslc4j/core/model

@berezovskyi
Copy link
Contributor

berezovskyi commented Oct 2, 2022

Happy to do that but only in Lyo 6.0. Can we mark them deprecated and fix the problem manually for now?

@berezovskyi
Copy link
Contributor

The conflict is between domains/oslc-domains/src/main/java/org/eclipse/lyo/oslc4j/core/model/package-info.java and core/oslc4j-core/src/main/java/org/eclipse/lyo/oslc4j/core/model/package-info.java.

I would actually prefer for 6.0 to:

  • separate lyo-core into lyo-core-base and lyo-core-model, maybe with a backward compat lyo-core POM package to import the two
  • remove org.eclipse.lyo.oslc4j.core.model package from oslc-domains, either by moving the Discussion into lyo-core-model, or removing it altogether.

For now, I think we can just remove the @OslcSchema annotation from domains/oslc-domains/src/main/java/org/eclipse/lyo/oslc4j/core/model/package-info.java and (if we plan to remove the Discussion instead of moving it, mark everything in domains/oslc-domains/src/main/java/org/eclipse/lyo/oslc4j/core/model @Deprecated)?

@jadelkhoury
Copy link
Contributor Author

remove the package-info.java class from model.

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

Successfully merging this pull request may close these issues.

2 participants