-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Use libraries-bom for snippets - part 2 #1781
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
Conversation
<!-- [END dependencies] --> | ||
<dependency> | ||
<groupId>junit</groupId> | ||
<artifactId>junit</artifactId> | ||
<version>4.12</version> | ||
<scope>test</scope> | ||
</dependency> | ||
<!-- [START dependencies] --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are intentional - the snippet is showing the following:
<dependencyManagement>
// bom stuff here
</dependencyManagement>
<dependencies>
<dependency>
// included dependency
</dependency>
// excluded dependencies
</dependencies>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's using the same tag name as above in the same file, does that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kurtisvg can you confirm this is ok to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - there are two ways to have "multi-part" snippets. You can use [START_EXCLUDE]
/[END_EXCLUDE]
or you can just end the tag and start it in the correct place (see http://go/code-snippets-201#including-code-from-multiple-sections-of-a-file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat, did not know that!
</dependencies> | ||
<!-- [END dependencies] --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
@@ -59,8 +69,9 @@ | |||
<version>4.13-beta-3</version> | |||
<scope>test</scope> | |||
</dependency> | |||
<!-- [START fs-maven] --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove these and check the rest of your changes for these weird tags, looks like a merge error.
@kurtisvg @dzlier-gcp I think all comments are addressed, mind taking another look? |
Towards #1761