Bug 32191 - Consistent upgrade messages
Summary: Consistent upgrade messages
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (command-line installer) (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-14 13:03 UTC by Tomás Cohen Arazi
Modified: 2022-11-17 08:31 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Bug 32191: Add sample output for copying and pasting to skeleton.pl (1.25 KB, patch)
2022-11-14 15:06 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32191: Tidy upgrade scripts output (61.03 KB, patch)
2022-11-14 15:06 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32191: Add sample output for copying and pasting to skeleton.pl (1.31 KB, patch)
2022-11-14 15:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32191: Tidy upgrade scripts output (60.98 KB, patch)
2022-11-14 15:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
update DB output 22.05.00 to 22.06.00.083 (11.01 KB, text/plain)
2022-11-15 08:27 UTC, Jonathan Druart
Details
22.05.00 => 22.06.83 upgrade (14.35 KB, text/plain)
2022-11-15 18:59 UTC, Tomás Cohen Arazi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2022-11-14 13:03:12 UTC
Our use of 'say $out'  is not really consistent, and we need to tidy it for the release.
Comment 1 Tomás Cohen Arazi 2022-11-14 15:06:06 UTC
Created attachment 143834 [details] [review]
Bug 32191: Add sample output for copying and pasting to skeleton.pl

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2022-11-14 15:06:12 UTC
Created attachment 143835 [details] [review]
Bug 32191: Tidy upgrade scripts output

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Katrin Fischer 2022-11-14 15:24:57 UTC
Looking here...
Comment 4 Katrin Fischer 2022-11-14 15:33:40 UTC
Created attachment 143838 [details] [review]
Bug 32191: Add sample output for copying and pasting to skeleton.pl

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2022-11-14 15:33:46 UTC
Created attachment 143839 [details] [review]
Bug 32191: Tidy upgrade scripts output

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2022-11-14 15:34:04 UTC
I <3 consistency!
Comment 7 Tomás Cohen Arazi 2022-11-14 18:09:28 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 8 Jonathan Druart 2022-11-15 08:25:31 UTC
Bug 30327 - Add biblionumber to ComponentSortField
  Added biblionumber option to ComponentSortField

Is this needed? What is the added value of the detail part?

I am not sure we are not adding confusion. For instance we have:
Bug 30392 - Add deleteditems.deleted_on

Without the "Added" part, which could mean it hasn't been added(?)

Also:
Bug 28269 - Add new system preference SearchWithISSNVariations
vs
Bug 30250 - Add new system preference ApplyFrameworkDefaults
  Added new system preference 'ApplyFrameworkDefaults'

We are lacking consistency.

My suggestion would be to:
1. Don't add a detail when there is only 1 thing to tell and it has already been said in the description
2. Don't add the past -ed, which is only useful when we are executing a script and want to provide feedback (here we only want to tell what it done, use infinitive everywhere then)
3. Use the detail page ($out) when there are several things. For instance:
Bug 42424: New awesome feature
  Add new DB column table.column
  Add new system preference SysPref
  etc.

Note that "Bug 32030 - Add an ERM module" could have a detail part.
Comment 9 Jonathan Druart 2022-11-15 08:27:32 UTC
Created attachment 143893 [details]
update DB output 22.05.00 to 22.06.00.083
Comment 10 Jonathan Druart 2022-11-15 08:41:03 UTC
Bug 31378 - Add identity_provider and identity_provider_domains configuration tables
	manage_identity_providers permission added

=> This is not consistent either.

It could be 

Bug 31378 - OAuth2/OIDC client implementation
  Add new DB tables identity_provider and identity_provider_domains
  Add new permission manage_identity_providers
Comment 11 Tomás Cohen Arazi 2022-11-15 18:57:35 UTC
(In reply to Jonathan Druart from comment #9)
> Created attachment 143893 [details]
> update DB output 22.05.00 to 22.06.00.083

I think that's before I pushed my patch, please try again or check the new attachment. It is pretty consistent to me right now. There might be style opinions, though. I basically renamed the descriptions for the bug descriptions and then put the 'actions' that took place, so the identity providers one really reads:

Upgrade to 22.06.00.081  [18:54:12]: Bug 31378 - Add identity_provider and identity_provider_domains configuration tables
	Added new permission 'manage_identity_providers'
	Added new table 'identity_providers'
	Added new table 'identity_provider_domains'
Comment 12 Tomás Cohen Arazi 2022-11-15 18:59:52 UTC
Created attachment 143922 [details]
22.05.00 => 22.06.83 upgrade
Comment 13 Jonathan Druart 2022-11-15 19:30:45 UTC
ah! I have certainly checked out master before the updatedatabase instead of the branch with this patch. That is way better now, thanks! Sorry for the confusion.
Comment 14 Katrin Fischer 2022-11-16 13:05:50 UTC
(In reply to Tomás Cohen Arazi from comment #11)
> (In reply to Jonathan Druart from comment #9)
> > Created attachment 143893 [details]
> > update DB output 22.05.00 to 22.06.00.083
> 
> I think that's before I pushed my patch, please try again or check the new
> attachment. It is pretty consistent to me right now. There might be style
> opinions, though. I basically renamed the descriptions for the bug
> descriptions and then put the 'actions' that took place, so the identity
> providers one really reads:
> 
> Upgrade to 22.06.00.081  [18:54:12]: Bug 31378 - Add identity_provider and
> identity_provider_domains configuration tables
> 	Added new permission 'manage_identity_providers'
> 	Added new table 'identity_providers'
> 	Added new table 'identity_provider_domains'

Should we add this to the wiki?
https://wiki.koha-community.org/wiki/Database_updates
Comment 15 Martin Renvoize 2022-11-16 15:13:03 UTC
Examples updated and added to on the wiki page.
Comment 16 Martin Renvoize 2022-11-16 15:13:32 UTC
Perhaps a QA script addition?
Comment 17 Tomás Cohen Arazi 2022-11-16 17:18:14 UTC
(In reply to Martin Renvoize from comment #16)
> Perhaps a QA script addition?

We should add it to some taiga board for 23.05. I cannot spend time on that right now.
Comment 18 Katrin Fischer 2022-11-16 18:37:16 UTC
(In reply to Tomás Cohen Arazi from comment #17)
> (In reply to Martin Renvoize from comment #16)
> > Perhaps a QA script addition?
> 
> We should add it to some taiga board for 23.05. I cannot spend time on that
> right now.

We didn't expect you to :) I've filed an issue on gitlab:
https://gitlab.com/koha-community/qa-test-tools/-/issues/63
Comment 19 Martin Renvoize 2022-11-17 08:31:56 UTC
Just to say, I love this consistency improvement.. another little polishing touch that makes Koha look all the more professional :-)