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

AF-1680: Add a Release Note entry about the Chart Library change #1538

Merged
merged 4 commits into from
May 23, 2019

Conversation

jesuino
Copy link
Contributor

@jesuino jesuino commented Mar 28, 2019

Add a Release Note entry about the Chart Library change and GChart removal in future release.

@jesuino
Copy link
Contributor Author

jesuino commented Mar 28, 2019

When the ticket AF-1680 was created GCharts was not marked for removal, hence why I would do a more detailed description about the libraries etc. Since GCharts will be removed (see https://issues.jboss.org/browse/AF-1774 )I think that a single Release Note is required to inform community users that GCharts will not be there in next release.


== New Chart API

Business Central uses a new API for charts rendering based on C3 and D3. The C3 Renderer API is used by default, now it implements all the features from the legacy chart library, GCharts, consequently GCharts is marked as deprecated and should be removed in later releases. To switch back to Google Charts you can use the system property `org.dashbuilder.renderer.default`, which should have the ID of the renderer you want to use by default. The possible values for Business Central are *gwtcharts* or *c3*, so to switch back to google use the system property `org.dashbuilder.renderer.default=gwtcharts`
Copy link
Contributor

@sterobin sterobin Mar 29, 2019

Choose a reason for hiding this comment

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

@jesuino, I suggest the following. Should someone else on the AppFormer team validate this note? Typically every PR should have both an SME and writer peer reviewer.

== New chart API for {CENTRAL}

{CENTRAL} uses a new API for chart rendering based on C3 and D3. The C3 Renderer API is used by default and implements all of the features from the previous chart library, Google Charts. The Google Charts library is now deprecated and will be removed in a future release. To revert to Google Charts,  you can set the renderer system property to `org.dashbuilder.renderer.default=gwtcharts`.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jesuino, just reminding you about this. An SME reviewer should also approve, per standard process with kie-docs PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sterobin Hello Stetson, I was waiting for a decision about what would be done with google charts renderer to update this doc. I will update it soon and ask a SME reviewer to approve this. Thanks.

@sterobin sterobin added the under review Label for PRs pending SME or peer review label Mar 29, 2019
@jesuino
Copy link
Contributor Author

jesuino commented May 21, 2019

Hello @paulovmr - would you please review my documentation change? I also added a README.md in gcharts project -> kiegroup/appformer@c32135d

@paulovmr paulovmr requested a review from sterobin May 21, 2019 15:12
@paulovmr
Copy link
Member

Hi @sterobin , could you please take a look? Thanks!

@sterobin
Copy link
Contributor

@paulovmr, @jesuino, please see my suggested rewrite in my previous comment in this PR. Looks like it was never implemented. If things have change in the text since then, I suggest adopting my rewrite or at least what you agree with, and then making the relevant tweaks to that rewrite.

@jesuino
Copy link
Contributor Author

jesuino commented May 22, 2019

Hello @sterobin I made the changes you requested. I also added a note in the dasboards section as requested by @ederign in kiegroup/appformer#705

Thanks and sorry for the confusion.

@@ -0,0 +1,7 @@
[[_jbpmreleasenotes72201]]

== New chart API for {CENTRAL}
Copy link
Contributor

Choose a reason for hiding this comment

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

@jesuino, you said you implemented my rewrite, but I don't see that you have completely. Still grammatical issues throughout. So to clarify, please use this entire rewrite below for this release note, which also now includes the bit that you added. If there is anything in this rewrite that is incorrect or that you disagree with, adjust as you see fit.

== New chart API for {CENTRAL}

{CENTRAL} uses a new API for chart rendering based on C3 and D3. The C3 Renderer API is used by default and implements all of the features from the previous chart library, Google Charts. The Google Charts library is now deprecated and will be removed in a future release. To revert to Google Charts,  build it from sources and add it to {CENTRAL} (see the `README.md` file in `dashbuilder-renderer-google`), and then set the renderer system property to `org.dashbuilder.renderer.default=gwtcharts`.


[NOTE]
====
By default Dashboards using C3/D3 libraries and Google Charts is deprecated and removed from default distribution. To switch back to Google Charts you have to build it from sources and add it to {CENTRAL} (see `README.md` in `dashbuilder-renderer-google`), then you can use the system property `org.dashbuilder.renderer.default`, which should have the ID of the renderer you want to use by default. For google charts you should use `org.dashbuilder.renderer.default=gwtcharts`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested rewrite, for clarity and consistency with the rewritten release note. Feel free to correct anything that is incorrect.

[NOTE]
====
By default, {CENTRAL} uses a C3 Renderer API for chart libraries and no longer uses the Google Charts library. To revert to Google Charts,  build it from sources and add it to {CENTRAL} (see the `README.md` file in `dashbuilder-renderer-google`), and then set the renderer system property to `org.dashbuilder.renderer.default=gwtcharts`.
====

Copy link
Contributor

@sterobin sterobin left a comment

Choose a reason for hiding this comment

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

@jesuino, the suggested changes/corrections were still not implemented. I've commented again inline to make sure we're clear on what needs to be changed. Also tweaked the new note.

@jesuino
Copy link
Contributor Author

jesuino commented May 23, 2019

@sterobin I hope this is correct now, let me know if I am missing anything, thanks!

@sterobin sterobin merged commit 17227f7 into apache:master May 23, 2019
kaldesai pushed a commit to kaldesai/kie-docs that referenced this pull request May 28, 2019
…che#1538)

* AF-1680: Add a Release Note entry about the Chart Library change and GChart removal in future release

* AF-1680: Updating Release Notes for jBPM 7.22.0

* AF-1680: Updating Release Notes and adding a note about the charts library change

* AF-1680: Improving note and release note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under review Label for PRs pending SME or peer review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants