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

Update osgi annotations in rest endpoint #310

Conversation

plutasnyy
Copy link
Contributor

I updated osgi annotations in rest endpoint

Motivation and Context

Before, annotations was marked as deprecated

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I hereby agree to the terms of the AET Contributor License Agreement.

@plutasnyy plutasnyy changed the base branch from master to feature/upgrade-osgi-annotations August 8, 2018 09:23
@plutasnyy plutasnyy force-pushed the update-osgi-annotations-rest-endpoint branch from 05e8a7a to 85c8504 Compare August 8, 2018 10:03
Copy link
Contributor

@malaskowski malaskowski left a comment

Choose a reason for hiding this comment

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

Code from this PR is not compiling. Travis log says:

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] Picked up _JAVA_OPTIONS: -Xmx2048m -Xms512m
/home/travis/build/Cognifide/aet/rest-endpoint/src/main/java/com/cognifide/aet/rest/helpers/ReportConfigurationManager.java:[18,51] error: package com.cognifide.aet.rest.helpers.configuration does not exist
[ERROR] /home/travis/build/Cognifide/aet/rest-endpoint/src/main/java/com/cognifide/aet/rest/helpers/ReportConfigurationManager.java:[27,2] error: cannot find symbol
  symbol:   class ReportConfigurationManagerConf
  location: class ReportConfigurationManager
[ERROR] /home/travis/build/Cognifide/aet/rest-endpoint/src/main/java/com/cognifide/aet/rest/helpers/ReportConfigurationManager.java:[30,23] error: cannot find symbol
  symbol:   class ReportConfigurationManagerConf
  location: class ReportConfigurationManager
[ERROR] /home/travis/build/Cognifide/aet/rest-endpoint/src/main/java/com/cognifide/aet/rest/helpers/ReportConfigurationManager.java:[24,17] error: cannot find symbol
  symbol: class ReportConfigurationManagerConf

@plutasnyy
Copy link
Contributor Author

My bad, I forgot add files to git repository ;)

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@Service
@Component(label = "MetadataServlet", description = "Returns Suite Metadata", immediate = true)
@Component(service = BasicDataServlet.class, immediate = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you check if service = HttpServlet.class would be enough here and in other BasicDataServlet extensions please? We only want this class to be registered as a servlet. It won't be referenced anywhere.

@@ -123,6 +123,7 @@

<build>
<plugins>
<!-- ToDo: remove me when all felix dependencies are removed -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clear also dependencies:

   <dependency>
      <groupId>org.apache.sling</groupId>
      <artifactId>org.apache.sling.commons.osgi</artifactId>
    </dependency>

and

    <dependency>
      <groupId>org.apache.felix</groupId>
      <artifactId>org.apache.felix.scr.annotations</artifactId>
    </dependency>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I edited wrong file, now should be correct

@PiteroS678 PiteroS678 force-pushed the update-osgi-annotations-rest-endpoint branch 3 times, most recently from baffb38 to 5374660 Compare August 10, 2018 08:09
@plutasnyy plutasnyy force-pushed the update-osgi-annotations-rest-endpoint branch from 5374660 to 34d8e54 Compare August 10, 2018 08:31
@plutasnyy plutasnyy merged commit 874ed9c into feature/upgrade-osgi-annotations Aug 10, 2018
@plutasnyy plutasnyy deleted the update-osgi-annotations-rest-endpoint branch August 10, 2018 13:19
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