Improved changelog groupings #105
Conversation
| - `/theaters` now returns a `application/mill.example.theater` Content-Type header on the following HTTP methods: | ||
| - `GET` | ||
| - `POST` | ||
| - The following Movies resources have changed: |
There was a problem hiding this comment.
I should designate "Movies" with the templating system I have so in Markdown this'll be wrapped in backticks, and in JSON, a mill-changelog styleable <span> element.
There was a problem hiding this comment.
I was just about to make that same comment (for all instances of Movies)
| - `/theaters` now returns a `application/mill.example.theater` Content-Type header on the following HTTP methods: | ||
| - `GET` | ||
| - `POST` | ||
| - The following Movies resources have changed: |
There was a problem hiding this comment.
I was just about to make that same comment (for all instances of Movies)
| /** | ||
| * Get a changelog entry for a changeset that was added into, or removed from, the API. | ||
| * | ||
| * @param string $definition |
There was a problem hiding this comment.
Might not be a problem when looking at the whole, but I needed to do some digging to find an example of a definition, It would be nice to have it a better explanation in these headers.
|
|
||
| $template = $this->templates['singular'][$definition]; | ||
| $entry[] = $this->renderText($template, [ | ||
| 'parameter' => rtrim(ltrim(array_shift($params), '`'), '`'), |
There was a problem hiding this comment.
pull this out into a descriptively named function call to allow for explanation without a comment line
There was a problem hiding this comment.
Now if only I could remember why I wrote that line originally...
There was a problem hiding this comment.
That line of code was actually from when before I had built the Markdown format generator, and it was currently untested. Slightly rewrote the code above it to only templatize $params if we need to, and added some comments explaining this to futureself.
| foreach ($data as /*$uri => */$change_types) { | ||
| foreach ($change_types as $change_type => $hashes) { | ||
| foreach ($hashes as /*$hash => */$changes) { | ||
| if ($definition === 'added' || $definition === 'removed') { |
| * @return string|array | ||
| * @throws \Exception If an unsupported definition + change type was supplied. | ||
| */ | ||
| private function getEntryForAddedOrRemovedChange($definition, $change_type, array $changes) |
There was a problem hiding this comment.
I feel like the word factory should be used somewhere around here :)
There was a problem hiding this comment.
Renaming to getAddedOrRemovedChangesetFactory and getChangedChangesetFactory.
| trait ChangelogTemplate | ||
| { | ||
| /** | ||
| * Changelog template output format. Acceptable values: "json", "html", or "markdown" |
There was a problem hiding this comment.
feels like those three vals should be const and referenced instead of compared to strings throughout
There was a problem hiding this comment.
Extracted them into:
Changelog::{DEFINITION_ADDED, DEFINITION_CHANGED, DEFINITION_REMOVED}
| $searches[] = '{' . $key . '}'; | ||
| if (is_array($value)) { | ||
| $replacements[] = $this->joinWords( | ||
| array_map(function ($val) use ($data_attribute_key, $key) { |
There was a problem hiding this comment.
maybe pull this out into a descriptively named function for legibility and to improve reading comprehension?
Also implemented some minor quality of life suggestions from @agilefox.
This makes some updates to the changelog generator to collapse similarly-grouped resources under resource headers, and split apart some other resource action method entries into their own entries.
The end result is hopefully a cleaner, and easier to comprehend changelog.
I also pulled out some changelog changeset code from god-like methods into their own classes. The changelog system as a whole should be easier to maintain now.