[WIP] Policy file per resource #1175
Conversation
services/publishing/Tweek.Publishing.Service/Sync/Converters/PolicyConverter.cs
Outdated
Show resolved
Hide resolved
services/publishing/Tweek.Publishing.Service/Sync/Converters/PolicyConverter.cs
Outdated
Show resolved
Hide resolved
| : `manifests/${resource.path}/policy.json`; | ||
| }; | ||
|
|
||
| export class PolicyRepository { |
There was a problem hiding this comment.
Why remove the default from the class?
Seems most places that import this file use only the class, so it makes sense for it to be the default
There was a problem hiding this comment.
I like more this style of exporting because if we'd like to export something else in the future from this file you won't have to update all the other exports.
There was a problem hiding this comment.
You can export a default along with more things, but it is a bit clunky to call it
| ); | ||
| addOid(this.context.response, oid); | ||
|
|
||
| return 'OK'; |
There was a problem hiding this comment.
Subjective, but I normally prefer not to include a response body if it does not add any information (aside from that the request succeeded, which is already indicated by the response status code).
If you don't return anything here, it will automatically return the 204 status code (which is for success without a request body)
There was a problem hiding this comment.
I'm with you but in all the other routes it's returning OK so I followed.
No description provided.