Skip to content

Add filename in the preview docs - #588

Merged
nickvergessen merged 1 commit into
masterfrom
filename-preview-docs
May 12, 2021
Merged

Add filename in the preview docs#588
nickvergessen merged 1 commit into
masterfrom
filename-preview-docs

Conversation

@MorrisJobke

Copy link
Copy Markdown
Member

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke

Copy link
Copy Markdown
MemberAuthor

I just wonder why this is added to the preview and not to the element itself 🤔

@MorrisJobke

Copy link
Copy Markdown
MemberAuthor

ref #576

@MorrisJobke

Copy link
Copy Markdown
MemberAuthor

@nickvergessen Tobias wondered yesterday if this should be seen as breaking change? There are JSON parsers out there that also break if new elements are added.

@nickvergessen

Copy link
Copy Markdown
Member

nickvergessen Tobias wondered yesterday if this should be seen as breaking change? There are JSON parsers out there that also break if new elements are added.

Adding additional items is per semver not a breaking change, so it should be fine. No need to duplicate the API unnecessarily.
Also any app could create a new object type and the clients should gracefully survive this.

I just wonder why this is added to the preview and not to the element itself 🤔

Because the preview object is disconnected from the files object (not every file can has a preview) and it was just easier this way to inject the data on the level it is needed and available instead of starting to map values later on.

@nickvergessen
nickvergessen merged commit 2352e67 into masterMay 12, 2021
@delete-merged-branch
delete-merged-branchBot deleted the filename-preview-docs branch May 12, 2021 19:41
@tobiasKaminsky

Copy link
Copy Markdown
Member

Adding additional items is per semver not a breaking change, so it should be fine. No need to duplicate the API unnecessarily.
Also any app could create a new object type and the clients should gracefully survive this.

ok, did not knew this. I already checked with other clients to see that everything is working.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@MorrisJobke@nickvergessen@tobiasKaminsky