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.

Sign up for GitHub

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

Jump to bottom

improved explanations on Maven Plugin vs Maven Site Integration/Doxia #322

Merged
merged 1 commit into from
Oct 29, 2017
Merged

improved explanations on Maven Plugin vs Maven Site Integration/Doxia #322

merged 1 commit into from
Oct 29, 2017

Conversation

hboutemy
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Oct 15, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 60.125% when pulling 896e509 on hboutemy:master into 3d44753 on asciidoctor:master.

@abelsromero
Copy link
Member

I added a couple of notes for simplification and avoiding repetition.

@hboutemy
Copy link
Contributor Author

great! but I don't see where you put these notes
Don't hesitate to improve content when merging

abelsromero
abelsromero requested changes Oct 18, 2017
View reviewed changes
README.adoc
@@ -50,7 +57,9 @@ If you're looking for the documentation for an older release, please refer to on
====
endif::[]

== Installation
== Maven Plugin
Copy link
Member

Choose a reason for hiding this comment

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

Plugin setup?

README.adoc Outdated
@@ -68,7 +77,7 @@ As this is a typical Maven plugin, simply declare the plugin in the `<plugins>`
----
<1> As this plugin tracks the version of Asciidoctor, you can use whichever version of Asciidoctor you prefer.

== Usage
=== Plugin Usage
Copy link
Member

Choose a reason for hiding this comment

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

just "Usage"?

README.adoc Outdated
== Installation
== Maven Plugin

=== Plugin Installation
Copy link
Member

Choose a reason for hiding this comment

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

Just "Installation"?

@abelsromero
Copy link
Member

I am still familiar with the new review model, seems I need to confirm my comments to make them visible. Should be visible now.

@abelsromero
Copy link
Member

Just checking @hboutemy can you see the comments now?

@coveralls
Copy link

coveralls commented Oct 25, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 60.125% when pulling 44a6a8e on hboutemy:master into 3d44753 on asciidoctor:master.

@hboutemy
Copy link
Contributor Author

sorry, I was busy on other tasks
improvements done, and more: now, each scenario has one "Setup" and one "Configuration" sections
this is a lot better IMHO: thank you for the feedback

@abelsromero
Copy link
Member

each scenario has one "Setup" and one "Configuration" sections

I have to say that I really like it.
Being non english native I use setup and config indistinctly, but I checked the dictionay and it fits.

I'll leave it open 24h in case someone else wants to say something, but it's fine for me.

@hboutemy
Copy link
Contributor Author

I like the result too: good team work :)

@abelsromero abelsromero merged commit 58e59a4 into asciidoctor:master Oct 29, 2017
hboutemy added a commit to hboutemy/asciidoctor.org that referenced this pull request Nov 12, 2017
@hboutemy
update page to match plugin's README
674b3ca 
particularly asciidoctor/asciidoctor-maven-plugin#322
@hboutemy hboutemy mentioned this pull request Nov 12, 2017
Merged
mojavelinux pushed a commit to asciidoctor/asciidoctor.org that referenced this pull request Nov 13, 2017
@hboutemy @mojavelinux
update page to match plugin's README (PR #743)
7dd4af5 
particularly asciidoctor/asciidoctor-maven-plugin#322
abelsromero pushed a commit to abelsromero/asciidoctor-maven-plugin that referenced this pull request Jan 7, 2018
@hboutemy @abelsromero
improved explanations on Maven Plugin vs Maven Site Integration/Doxia (…
e4ede84 
…asciidoctor#322)
@abelsromero abelsromero added this to the 1.5.7 milestone Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abelsromero abelsromero abelsromero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.5.7
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hboutemy @coveralls @abelsromero