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

Don't add revnumber attr when project.version is unspecified #512

Conversation

lhotari
Copy link
Contributor

@lhotari lhotari commented Jan 24, 2020

@@ -770,7 +771,7 @@ abstract class AbstractAsciidoctorBaseTask extends DefaultTask {
'gradle-project-name': (Object) project.name
]

if (project.version != null) {
if (project.version != null && project.version.toString() != Project.DEFAULT_VERSION) {
Copy link
Member

Choose a reason for hiding this comment

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

EMI, but what does project.DEFAULT_VERSION provide ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ysb33r
Copy link
Member

ysb33r commented Jan 24, 2020

Hey @lhotari Can you base your PR against development-3.x, please?

@lhotari
Copy link
Contributor Author

lhotari commented Jan 24, 2020

Hey @lhotari Can you base your PR against development-3.x, please?

Yes. working on it.

@lhotari lhotari changed the base branch from master to development-3.x January 24, 2020 18:32
@lhotari lhotari force-pushed the lh-fix-revnumber-added-when-project.version-unspecified branch from 202e5bf to 191d4df Compare January 24, 2020 18:32
@lhotari
Copy link
Contributor Author

lhotari commented Jan 24, 2020

It's rebased now for development-3.x. I also added another fix in this same PR that I came across at the same time. I can move that to a separate PR if that's required. The 2nd commit doesn't add revnumber@ when it has been specified by the user.

@ysb33r
Copy link
Member

ysb33r commented Jan 24, 2020

I can move that to a separate PR if that's required.

Keep in the same one. There is enough material being contributed such that we can release 3.1.0 shortly. // cc @Mogztter

@ysb33r
Copy link
Member

ysb33r commented Jan 25, 2020

@lhotari There still seem to be one test failure.

@ysb33r ysb33r added this to the 3.1.0 milestone Jan 25, 2020
@ysb33r ysb33r added 3.x Issues related to the 3.x bug labels Jan 25, 2020
@lhotari
Copy link
Contributor Author

lhotari commented Jan 25, 2020

There still seem to be one test failure.

will check it now.

- Fixes asciidoctor#329
- project.version defaults to 'unspecified' in Gradle
@lhotari lhotari force-pushed the lh-fix-revnumber-added-when-project.version-unspecified branch from 191d4df to 2a3f882 Compare January 25, 2020 20:00
@lhotari
Copy link
Contributor Author

lhotari commented Jan 25, 2020

@ysb33r I pushed a fix for the test failure, https://github.com/asciidoctor/asciidoctor-gradle-plugin/pull/512/files#diff-5479704be335de86c1fe2110ef764cbbR1033-R1040 . Had to use CompileDynamic to resolve the issue. It seems that the signature of .collect changed in Groovy 2.5.0 .

private Set<String> trimOverridableAttributeNotation(Set<String> attributeKeys) {
// remove possible trailing '@' character that is used to encode that the attribute can be overridden
// in the document itself
attributeKeys.collect { k -> k - ~/@$/ } as Set
Copy link
Member

Choose a reason for hiding this comment

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

We have a utility to deal with that which will avoid dynamic compilation.

Transform.toSet (attributeKeys) { k -> k - ~/@$/ }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. good to know. Using Transform.toSet now.

- internal default for 'revnumber@' should not override the
  one provided by the user
@lhotari lhotari force-pushed the lh-fix-revnumber-added-when-project.version-unspecified branch from 2a3f882 to f6a41a7 Compare January 26, 2020 05:21
@ysb33r ysb33r merged commit 3b7dca7 into asciidoctor:development-3.x Jan 26, 2020
@ysb33r ysb33r mentioned this pull request Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Issues related to the 3.x bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants