Skip to content

Comments

Fix sdk/spring/README.md not found error in non-spring ci pipelines#29013

Merged
chenrujun merged 5 commits intoAzure:mainfrom
saragluna:xiada/fix-readme-not-found-in-nonspring-ci
May 23, 2022
Merged

Fix sdk/spring/README.md not found error in non-spring ci pipelines#29013
chenrujun merged 5 commits intoAzure:mainfrom
saragluna:xiada/fix-readme-not-found-in-nonspring-ci

Conversation

@saragluna
Copy link
Member

@saragluna saragluna commented May 23, 2022

Description

We added the empty Javadoc and sources jar files in #28988, but it will use the README file in sdk/spring folder, which will be an issue when build from source pipelines run in the non-spring service directory, for the sparse checkout won't check out the sdk/spring folder, like this failure. This PR will fix this issue by disabling throwing exceptions when the file can't be found. This PR will also fix the bug of not cleaning the sourceTemp folder for each run.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@ghost ghost added the azure-spring All azure-spring related issues label May 23, 2022
@saragluna saragluna requested a review from XiaofeiCao May 23, 2022 06:58
@saragluna
Copy link
Member Author

/azp run java - resourcemanager - ci

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@saragluna saragluna added this to the [2022] June milestone May 23, 2022
@saragluna saragluna requested a review from hallipr as a code owner May 23, 2022 07:20
@saragluna
Copy link
Member Author

/azp run java - resourcemanager - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@saragluna
Copy link
Member Author

The resource manager build passed.

@XiaofeiCao
Copy link
Contributor

Thanks!

${project.basedir}/javadocTemp/README.md
</echo>
<copy file="${project.basedir}/../README.md" tofile="${project.basedir}/javadocTemp/README.md"/>
<copy file="${project.basedir}/../README.md" tofile="${project.basedir}/javadocTemp/README.md" failonerror="false"/>

Choose a reason for hiding this comment

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

Maybe we should update the sparse checkout instead of setting failonerror="false" here.

Setting failonerror="false" may cause the released javadoc / sourcejar does not contains README.md.

We should avoid uncertain behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's okay, the sdk/spring/REAME.md will always be there, and what we want to achieve here is to avoid exceptions when sparse checkout does not include the sdk/spring/README.md file.

Copy link
Member Author

Choose a reason for hiding this comment

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

What we also want to avoid is adding a README.md in each starter's folder.

Copy link
Member Author

Choose a reason for hiding this comment

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

And even if we ship some Javadoc and source jar files without the README, I think it'll still be ok. The starters don't have any source code or Javadoc.

Copy link

@chenrujun chenrujun May 23, 2022

Choose a reason for hiding this comment

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

The key point is We should avoid uncertain behavior.

Consider this conversation:

Customer: Does the source jar contains README?
We: We are not sure.

That's not professional.

Copy link

@chenrujun chenrujun left a comment

Choose a reason for hiding this comment

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

I'll approve this PR to quick fix pipeline failure.

But whether we should contain s README.md in source-jar and java-doc-jar need more discussion

@saragluna
Copy link
Member Author

@chenrujun, you can help create another PR to remove the README file from the Javadoc and sources jar files.

@chenrujun chenrujun merged commit 1fc5055 into Azure:main May 23, 2022
@saragluna saragluna deleted the xiada/fix-readme-not-found-in-nonspring-ci branch May 23, 2022 09:19
@saragluna
Copy link
Member Author

saragluna commented May 23, 2022

But I think this will provide more context on what's a usable Javadoc jar file, like this https://azuresdkdocs.blob.core.windows.net/$web/java/azure-storage-blob/12.16.1/index.html.

I have no preference for whether our starter Javadoc jar must have a README.md file or not, but if removing the README logic could simplify our pom files, it would be great and worth a try.

chenrujun pushed a commit to chenrujun/azure-sdk-for-java that referenced this pull request May 23, 2022
@saragluna saragluna self-assigned this May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

azure-spring All azure-spring related issues

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants