Bug 29072 - Move reference route /cities spec to YAML
Summary: Move reference route /cities spec to YAML
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 28189
Blocks: 29620
  Show dependency treegraph
 
Reported: 2021-09-21 12:39 UTC by Tomás Cohen Arazi
Modified: 2022-06-06 20:31 UTC (History)
4 users (show)

See Also:
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


Attachments
Bug 29072: Move reference route /cities spec to YAML (17.45 KB, patch)
2021-09-21 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29072: Move reference route /cities spec to YAML (17.52 KB, patch)
2021-09-21 13:03 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.