Login  Register

Please test HEAD from asciidoctor-maven-plugin

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options Options
Embed post
Permalink
Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Please test HEAD from asciidoctor-maven-plugin

LightGuardjp
I'm at the point where I think version 0.1.4 is ready to go. You can see the list of https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=7&page=1&state=closed[issues fixed]. I've had to move some that were slated for 0.1.4 to https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=8&page=1&state=open[1.5].

I'm asking people to test this version with your projects and try things.

[NOTE]
====
* `sourceDirectory` must be set, but still contains the default of `${basedir}src/main/asciidoc`
* `baseDir` currently doesn't do anything anymore.
====

I'm hoping everything works as people expect it to.

Jmm
Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Re: Please test HEAD from asciidoctor-maven-plugin

Jmm
Hello,

I tried the HEAD version on my projects and I get a "all lights green".

Not sure I understood your note correctly; I didn't use the default source source directory.

Would you like some specific validations ?

Jmm


2013/12/22 LightGuardjp [via Asciidoctor :: Discussion] <[hidden email]>
I'm at the point where I think version 0.1.4 is ready to go. You can see the list of https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=7&page=1&state=closed[issues fixed]. I've had to move some that were slated for 0.1.4 to https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=8&page=1&state=open[1.5].

I'm asking people to test this version with your projects and try things.

[NOTE]
====
* `sourceDirectory` must be set, but still contains the default of `${basedir}src/main/asciidoc`
* `baseDir` currently doesn't do anything anymore.
====

I'm hoping everything works as people expect it to.




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Please-test-HEAD-from-asciidoctor-maven-plugin-tp1217.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML

Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Re: Please test HEAD from asciidoctor-maven-plugin

LightGuardjp
I'm saying it has a default already, which is src/main/asciidoc. If you have files in a different location than that you will need to set the value. 

Good to hear it's working well for projects in the wild. A few more green lights and I'll call it good and push it out. 

Sent from Mailbox for iPhone


On Sat, Dec 21, 2013 at 10:38 PM, Jmm [via Asciidoctor :: Discussion] <[hidden email]> wrote:

Hello,

I tried the HEAD version on my projects and I get a "all lights green".

Not sure I understood your note correctly; I didn't use the default source source directory.

Would you like some specific validations ?

Jmm


2013/12/22 LightGuardjp [via Asciidoctor :: Discussion] <[hidden email]>
I'm at the point where I think version 0.1.4 is ready to go. You can see the list of https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=7&page=1&state=closed[issues fixed]. I've had to move some that were slated for 0.1.4 to https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=8&page=1&state=open[1.5].

I'm asking people to test this version with your projects and try things.

[NOTE]
====
* `sourceDirectory` must be set, but still contains the default of `${basedir}src/main/asciidoc`
* `baseDir` currently doesn't do anything anymore.
====

I'm hoping everything works as people expect it to.




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Please-test-HEAD-from-asciidoctor-maven-plugin-tp1217.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Please-test-HEAD-from-asciidoctor-maven-plugin-tp1217p1218.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML

Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Re: Please test HEAD from asciidoctor-maven-plugin

mojavelinux
Administrator
In reply to this post by LightGuardjp
Jason,

I did some testing and overall it looks good. I sent a pull request to make a fix to the embedAssets setting that I think should be merged in before release.

The default for baseDir works for now. Your note about it is incorrect, though. It does do something. It sets the root for includes. Using the AsciiDoc directory (sourceDirectory) as the default seems to make sense, esp since it can still be overridden.

We'll probably do some more thinking about these paths for 1.5.0, especially since core should be changing to make the behavior more consistent.

I'm a little concerned that too many attributes are becoming top-level configuration parameters in the plugin. For instance, imagesdir should probably remain an attribute, not a configuration property. Also, the default of "images" changes the default behavior of Asciidoctor. It's not a big deal, but I think that being consistent with the default value for this attribute is a good idea.

I'm also still annoyed that Maven doesn't allow you to set a property to empty string. I want to set idprefix to empty string (e.g., <idprefix></idprefix>) to effectively disable it, but Maven won't accept a blank value for a property. I think in general we should figure out how to specify an empty string value and check for that value across all attributes (similar to what we did for the boolean attributes).

With the fix to embedAssets, I think we can move forward. Nice work.

-Dan


On Sat, Dec 21, 2013 at 8:50 PM, LightGuardjp [via Asciidoctor :: Discussion] <[hidden email]> wrote:
I'm at the point where I think version 0.1.4 is ready to go. You can see the list of https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=7&page=1&state=closed[issues fixed]. I've had to move some that were slated for 0.1.4 to https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=8&page=1&state=open[1.5].

I'm asking people to test this version with your projects and try things.

[NOTE]
====
* `sourceDirectory` must be set, but still contains the default of `${basedir}src/main/asciidoc`
* `baseDir` currently doesn't do anything anymore.
====

I'm hoping everything works as people expect it to.




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Please-test-HEAD-from-asciidoctor-maven-plugin-tp1217.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--
Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Re: Please test HEAD from asciidoctor-maven-plugin

mojavelinux
Administrator
In reply to this post by LightGuardjp
Btw, I created a sample Maven project that demonstrates the use of this plugin. We need to figure out the best place to stick it. Perhaps it's best to put it inside the plugin's repository.


Thinking aloud, we should create two profiles, one that using embedAssets and one that copies the assets to the output directory.

-Dan


On Sun, Dec 22, 2013 at 8:51 PM, Dan Allen <[hidden email]> wrote:
Jason,

I did some testing and overall it looks good. I sent a pull request to make a fix to the embedAssets setting that I think should be merged in before release.

The default for baseDir works for now. Your note about it is incorrect, though. It does do something. It sets the root for includes. Using the AsciiDoc directory (sourceDirectory) as the default seems to make sense, esp since it can still be overridden.

We'll probably do some more thinking about these paths for 1.5.0, especially since core should be changing to make the behavior more consistent.

I'm a little concerned that too many attributes are becoming top-level configuration parameters in the plugin. For instance, imagesdir should probably remain an attribute, not a configuration property. Also, the default of "images" changes the default behavior of Asciidoctor. It's not a big deal, but I think that being consistent with the default value for this attribute is a good idea.

I'm also still annoyed that Maven doesn't allow you to set a property to empty string. I want to set idprefix to empty string (e.g., <idprefix></idprefix>) to effectively disable it, but Maven won't accept a blank value for a property. I think in general we should figure out how to specify an empty string value and check for that value across all attributes (similar to what we did for the boolean attributes).

With the fix to embedAssets, I think we can move forward. Nice work.

-Dan


On Sat, Dec 21, 2013 at 8:50 PM, LightGuardjp [via Asciidoctor :: Discussion] <[hidden email]> wrote:
I'm at the point where I think version 0.1.4 is ready to go. You can see the list of https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=7&page=1&state=closed[issues fixed]. I've had to move some that were slated for 0.1.4 to https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=8&page=1&state=open[1.5].

I'm asking people to test this version with your projects and try things.

[NOTE]
====
* `sourceDirectory` must be set, but still contains the default of `${basedir}src/main/asciidoc`
* `baseDir` currently doesn't do anything anymore.
====

I'm hoping everything works as people expect it to.




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Please-test-HEAD-from-asciidoctor-maven-plugin-tp1217.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--



--
Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Re: Please test HEAD from asciidoctor-maven-plugin

LightGuardjp
In reply to this post by mojavelinux


On Sun, Dec 22, 2013 at 18:54, mojavelinux [via Asciidoctor :: Discussion] <[hidden email]="mailto:[hidden email]">> wrote:
Jason,

I did some testing and overall it looks good. I sent a pull request to make a fix to the embedAssets setting that I think should be merged in before release.
Saw that, happy to merge in tomorrow. 

The default for baseDir works for now. Your note about it is incorrect, though. It does do something. It sets the root for includes. Using the AsciiDoc directory (sourceDirectory) as the default seems to make sense, esp since it can still be overridden.

We'll probably do some more thinking about these paths for 1.5.0, especially since core should be changing to make the behavior more consistent.
That may be, wasn't looking at includes, but embeds, which as to our phone conversation, is probably one of those areas of inconsistency in core. 

I'm happy with where it is now, and should be less surprising for people, unless you're doing embedding, which is still partially broken, unless your PR fixes those. 

I'm a little concerned that too many attributes are becoming top-level configuration parameters in the plugin. For instance, imagesdir should probably remain an attribute, not a configuration property. Also, the default of "images" changes the default behavior of Asciidoctor. It's not a big deal, but I think that being consistent with the default value for this attribute is a good idea.
Feeling the same way, and I think it's worth dialing back in 1.5.0 for AsciidoctorJ and the plugins. I think AsciidoctorJ can do the looping and fixing of attributes and options instead of making them top level. 

IMO the only things that should be top level are things we make top level in core CLI usage. Thoughts there?

I'm also still annoyed that Maven doesn't allow you to set a property to empty string. I want to set idprefix to empty string (e.g., <idprefix></idprefix>) to effectively disable it, but Maven won't accept a blank value for a property. I think in general we should figure out how to specify an empty string value and check for that value across all attributes (similar to what we did for the boolean attributes).
+1

With the fix to embedAssets, I think we can move forward. Nice work.

-Dan


On Sat, Dec 21, 2013 at 8:50 PM, LightGuardjp [via Asciidoctor :: Discussion] <[hidden email]> wrote:
I'm at the point where I think version 0.1.4 is ready to go. You can see the list of https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=7&page=1&state=closed[issues fixed]. I've had to move some that were slated for 0.1.4 to https://github.com/asciidoctor/asciidoctor-maven-plugin/issues?milestone=8&page=1&state=open[1.5].

I'm asking people to test this version with your projects and try things.

[NOTE]
====
* `sourceDirectory` must be set, but still contains the default of `${basedir}src/main/asciidoc`
* `baseDir` currently doesn't do anything anymore.
====

I'm hoping everything works as people expect it to.




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Please-test-HEAD-from-asciidoctor-maven-plugin-tp1217.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--



If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/Please-test-HEAD-from-asciidoctor-maven-plugin-tp1217p1220.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML
Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Re: Please test HEAD from asciidoctor-maven-plugin

LightGuardjp
In reply to this post by mojavelinux
+1 Send pull requset
Reply | Threaded
Open this post in threaded view
| More
Print post
Permalink

Re: Please test HEAD from asciidoctor-maven-plugin

LightGuardjp
In reply to this post by LightGuardjp
Merged. I think we're ready to release, right?