Bug 29072

Summary: Move reference route /cities spec to YAML
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: minor    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, kyle, martin.renvoize
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.05
Bug Depends on: 28189    
Bug Blocks: 29620    
Attachments: Bug 29072: Move reference route /cities spec to YAML
Bug 29072: Move reference route /cities spec to YAML

Description Tomás Cohen Arazi 2021-09-21 12:39:51 UTC
We have been adding routes in YAML format, which provides an easy path for adding Markdown documentation. The reference route should be migrated.
Comment 1 Tomás Cohen Arazi 2021-09-21 12:54:59 UTC
Created attachment 125105 [details] [review]
Bug 29072: Move reference route /cities spec to YAML

This patch moves the /cities routes spec and related files into YAML so
devs can use it as a reference for future routes.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Martin Renvoize 2021-09-21 13:03:47 UTC
Created attachment 125106 [details] [review]
Bug 29072: Move reference route /cities spec to YAML

This patch moves the /cities routes spec and related files into YAML so
devs can use it as a reference for future routes.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2021-09-21 13:05:16 UTC
Nice One!

This is great, I'll stop defaulting to JSON now (as I often use this as a template for creating other new routes!)

All works before and after patch, tests passing, QA script happy..

Going for a straight PQA again here!  It's only a string patch really.
Comment 4 Jonathan Druart 2021-09-21 18:19:21 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 5 Kyle M Hall 2021-10-01 18:27:26 UTC
Pushed to 21.05.x for 21.05.05
Comment 6 Fridolin Somers 2021-10-06 20:31:05 UTC
Depends on Bug 28189 not in 20.11.x
Comment 7 Fridolin Somers 2021-10-09 02:16:03 UTC
Depends on Bug 28189 not in 20.11.x
Comment 8 Tomás Cohen Arazi 2021-10-09 15:58:22 UTC
(In reply to Fridolin Somers from comment #6)
> Depends on Bug 28189 not in 20.11.x

It wouldn't hurt to backported both, for the sake of better documentation. I'm willing to generate a revised patch if you would push it
Comment 9 Fridolin Somers 2021-10-11 22:04:23 UTC
Bug 28189 imself depends on Bug 28157.
With new cycle about to start, I'm not sure it is worth spending the time on it.
Better keep 20.11 as stable as possible in my opinion.