search

SasaKaranovic / VU-Server

VU Dials Server Application
https://vudials.com
26 stars 12 forks source link

Fix force image set #37

Closed homeworkprod closed 4 months ago

homeworkprod commented 6 months ago

See commit message for details.

homeworkprod commented 6 months ago

Note: This is considering version 6.4 of Tornado, which is the current version at this time and the one that got installed as the requirements file does not limit or pin versions.

github-actions[bot] commented 5 months ago

This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days.

homeworkprod commented 4 months ago

@SasaKaranovic would you mind taking a look at this? It fixes an issue I've encountered.

SasaKaranovic commented 4 months ago

Hey @homeworkprod Sorry about that.

How are you using this parameter? Maybe updating the documentation is more appropriate.

Reason I'm suggesting this is that this parameter should "almost never" be used. It is a fail-safe to force the server to update the dials image, even if it's certain that the dial already contains the exact same image and therefore no update is required.

So in normal operation you would completely omit this parameter. And if you need to use it, you would just set it, the value is ignored.

Also in the future, opening an issue first is probably better way to do this. I should add a contribution guide so we can guide people better in the future.

homeworkprod commented 4 months ago

Ok, so you're saying the query parameter is supposed to have an effect when it is set, regardless of its value? Then indeed making that clear in the docs would've helped me.

How are you using this parameter?

It's been two months and I can't remember why I wanted to, and did, use it, but I guess I had some reason. I'll let you know once I remember it or encounter such a situation again. No need to investigate this further right know, I think.

Also in the future, opening an issue first is probably better way to do this.

Fair point. I try to reduce noise/overhead a bit by often going straight to a PR, but this does have drawbacks.

SasaKaranovic commented 4 months ago

That is correct. Just setting the parameter is sufficient. And this parameter should not be used under "normal operation", so I will document it better.

Fair point. I try to reduce noise/overhead a bit by often going straight to a PR, but this does have drawbacks.

Just to clarify, it takes time and effort to make a PR! With that said I really appreciate all the help that people want to offer! The "issue first, PR later" approach helps avoid wasted work and also allows everyone to get aligned before any work is kicked off.