fix: add PM properties to HTTP accessor #1228
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/R51-live-status-gateway-on-R50 #1228 +/- ##
=======================================================================
+ Coverage 57.95% 57.98% +0.02%
=======================================================================
Files 485 485
Lines 79860 79864 +4
Branches 3547 3598 +51
=======================================================================
+ Hits 46280 46306 +26
+ Misses 33543 33536 -7
+ Partials 37 22 -15 ☔ View full report in Codecov by Sentry. |
| {t( | ||
| 'Set to true if the HTTP server supports HEAD requests. (otherwise GET requests will be used to check availability)' | ||
| )} |
There was a problem hiding this comment.
Any reasonable HTTP server should support HEAD in 2024 - can we switch the default around? To say, "Use GET requests instead of HEAD to check availability"?
express.js generates a HEAD handler automatically for GET routes: https://expressjs.com/en/api.html#app.METHOD - I think so does koa-router, so I think it's safe to assume that HEAD is supported, unless it isn't, not the other way round.
There was a problem hiding this comment.
Yeah, that's a good point, that would also be solve a backwards compatibility issue.
| /** path to the HTML template */ | ||
| path: string | ||
| /** Add prefix to output artifacts */ | ||
| outputPrefix: string |
There was a problem hiding this comment.
What does this add a prefix to? the url filename of the artifact?
There was a problem hiding this comment.
Yes, the output of this package are defined by the various defined steps. This adds a prefix to them. Hmm, perhaps we should remove this altogether..?
| /** Zoom level, defaults to 1 */ | ||
| zoom?: number | ||
| /** (defaults to black) */ | ||
| backgroundColor?: string |
There was a problem hiding this comment.
How is this color specified? CSS Colors? Can this be set to transparent? Can this be background and not backgroundColor so that this is set to a background-image of choice?
| // Store an object in memory | ||
| | { do: 'storeObject'; key: string; value: Record<string, any> } | ||
| // Modify an object in memory. Path is a dot-separated string | ||
| | { do: 'modifyObject'; key: string; path: string; value: any } | ||
| // Send an object to the renderer as a postMessage | ||
| | { do: 'injectObject'; key: string } |
There was a problem hiding this comment.
I find the naming here a bit confusing - these actions are not related - storeObject and injectObject? Since there is no retrieveObject, maybe it would make more sense to call it setObject or even setValue if there's no reason to limit this to objects? and then change injectObject into postMessage, since that's what's going to happen?
Also I suggest not saying "memory" and instead say "page context".
| // Store an object in memory | |
| | { do: 'storeObject'; key: string; value: Record<string, any> } | |
| // Modify an object in memory. Path is a dot-separated string | |
| | { do: 'modifyObject'; key: string; path: string; value: any } | |
| // Send an object to the renderer as a postMessage | |
| | { do: 'injectObject'; key: string } | |
| // Set a value in renderer's global context | |
| | { do: 'setValue'; key: string; value: any } | |
| // Modify an object in renderer's global context. Path is a dot-separated string | |
| | { do: 'modifyObject'; key: string; path: string; value: any } | |
| // Send an object to the renderer as a postMessage | |
| | { do: 'postMessage'; value: string } |
|
superseded by #1247 |
About the Contributor
This pull request is posted on behalf of the NRK
Type of Contribution
This is a:
Feature
New Behavior
Adds types for the new Package Manager HTML-Template rendering
Testing
Affected areas
This PR only adds types and minor additions in GUI settings, does not affect much else.
Status