search

miraisolutions / SmaRP

Shiny app for projecting retirement funds / benefits
https://mirai-solutions.ch/gallery/smarp
20 stars 9 forks source link

Fix/96 plot in report #98

Closed fvitalini closed 5 years ago

fvitalini commented 5 years ago
riccardoporreca commented 5 years ago

@fvitalini, I think we should be more explicit about what was done here.

The main driver was fixing bug #96.

On top of this, plot labels were changed from Occupational/Private Pension to Occupational/Private Fund in both the app and report, which were aligned. The renaming was probably wanted (@gabrielfoix?), but it does not quite belong to the bugifx and its branch (strictly speaking).

What is definitely missing is mentioning bugfix #96 in NEWS.md, which should rather read as something like

* Fixed missing bar-plot in the PDF report (#6).
* Reviewed plots labels.

Actually, naming probably deserves its own issue, since we are not very consistent between plots, table, text in the report. I opened a dedicated issue #103, we might want to revert the labels change done here and delegate it to it.

fvitalini commented 5 years ago

@fvitalini, I think we should be more explicit about what was done here.

The main driver was fixing bug #96.

On top of this, plot labels were changed from Occupational/Private Pension to Occupational/Private Fund in both the app and report, which were aligned. The renaming was probably wanted (@gabrielfoix?), but it does not quite belong to the bugifx and its branch (strictly speaking).

What is definitely missing is mentioning bugfix #96 in NEWS.md, which should rather read as something like

* Fixed missing bar-plot in the PDF report (#6).
* Reviewed plots labels.

Actually, naming probably deserves its own issue, since we are not very consistent between plots, table, text in the report. I opened a dedicated issue #103, we might want to revert the labels change done here and delegate it to it.

@riccardoporreca In commit 32dadde I have updated the NEWS.md according to your suggestion.