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

remove profile that disables doclint #3225

Closed
wants to merge 1 commit into from

Conversation

neozwu
Copy link
Contributor

@neozwu neozwu commented May 2, 2018

Remove this piece of magic code that automatically activates the profile and disables javadoc lint check. Any javadoc errors will still bite us during releasing where a different profile is activated.

@neozwu neozwu requested a review from pongad as a code owner May 2, 2018 19:21
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 2, 2018
@igorbernstein2
Copy link

Why does activating another profile disable this one?

@igorbernstein2
Copy link

igorbernstein2 commented May 2, 2018

for example, mvn -Pbaa compile still prints hi using the below pom.xml

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.mycompany.app</groupId>
   <artifactId>my-app</artifactId>
   <version>1</version>
   <profiles>
      <profile>
         <id>moo</id>
         <activation>
            <jdk>[1.8,)</jdk>
         </activation>
         <build>
            <plugins>
               <plugin>
                  <groupId>com.github.ekryd.echo-maven-plugin</groupId>
                  <artifactId>echo-maven-plugin</artifactId>
                  <version>1.2.0</version>
                  <executions>
                     <execution>
                        <phase>compile</phase>
                        <goals>
                           <goal>echo</goal>
                        </goals>
                        <configuration>
                           <message>hi!</message>
                        </configuration>
                     </execution>
                  </executions>
               </plugin>
            </plugins>
         </build>
      </profile>
      <profile>
         <id>baa</id>
      </profile>
   </profiles>
</project>

@neozwu
Copy link
Contributor Author

neozwu commented May 2, 2018

@igorbernstein2 I think you are right. Multiple profiles can be activated at same time. We had this issue - in circle CI docker image, mvn javadoc:jar won't throw errors (only warnings). But those errrors will be thrown during release process. I thought it had something to do with profiles because release uses "release" profile. I took a second look and this issue doesn't seem to be related to profiles. Nevertheless, I still would like to remove this profile unless there's a reason to keep it.

@igorbernstein2
Copy link

No worries. I'll just move it to my ~/.m2/settings.xml

@JustinBeckwith
Copy link
Contributor

Greetings folks! This issue is getting a little moldy. Should we drive this one forward, or close it out?

@igorbernstein2
Copy link

My original attempt to disable javadoc lint got lost when the directory structure changed. I'm planning on re-introducing the change after reporting change gets unreverted:

#3524 (comment)

To clarify my intent: I would like to disable all java8 doclint warnings about missing parameter docs. This is similar to what gax does:
https://github.com/googleapis/gax-java/blob/master/build.gradle#L186

However in maven this is bit uglier because I have use a profile to conditionally target java8 only

@chingor13
Copy link
Contributor

We can do this by updating to maven-javadoc-plugin 3.0+ and set the doclint configuration option. Adding a work item to #3757

@chingor13 chingor13 closed this Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants