Yooooomi / your_spotify

Self hosted Spotify tracking dashboard
GNU General Public License v3.0
2.64k stars 108 forks source link

Fixed some minor typos in various files; fixed CORS verbiage #402

Closed kmanwar89 closed 1 month ago

kmanwar89 commented 1 month ago
RagingCactus commented 1 month ago

Speaking of CORS: Can we try to not push people to allow CORS on all origins in the documentation and examples? The current (and the proposed) templates seem to suggest to people that this is required to be set to the insecure value in most cases. This is incorrect, the automatic defaults should be good enough in the deployment scenarios YourSpotify is usually used in, so no manual CORS settings are required at all. In my personal deployment I have not configured CORS manually and it works perfectly.

The results of the current documentation and examples can be seen in the issues, where basically everyone thinks their issues are cause by CORS and in all cases I've seen it turns out that either a reverse proxy is configured inccorectly or the backend/frontend URLs are not set correctly.

I would therefore suggest to remove the CORS option from the examples altogether.

kmanwar89 commented 1 month ago

Speaking of CORS: Can we try to not push people to allow CORS on all origins in the documentation and examples? The current (and the proposed) templates seem to suggest to people that this is required to be set to the insecure value in most cases. This is incorrect, the automatic defaults should be good enough in the deployment scenarios YourSpotify is usually used in, so no manual CORS settings are required at all. In my personal deployment I have not configured CORS manually and it works perfectly.

The results of the current documentation and examples can be seen in the issues, where basically everyone thinks their issues are cause by CORS and in all cases I've seen it turns out that either a reverse proxy is configured inccorectly or the backend/frontend URLs are not set correctly.

I would therefore suggest to remove the CORS option from the examples altogether.

I've no argument for or against it; I just wanted to propose that the documentation be updated to reflect the current option. Whether or not someone chooses to use it is up to them, and requiring a very long string is probably a good, in-your-face, this isn't a good idea but if you want to do it, here's how type of configuration.

RagingCactus commented 1 month ago

I've no argument for or against it; I just wanted to propose that the documentation be updated to reflect the current option. Whether or not someone chooses to use it is up to them, and requiring a very long string is probably a good, in-your-face, this isn't a good idea but if you want to do it, here's how type of configuration.

My suggestion would be to leave it in the documentation (although maybe with an even more clear hint) but remove it from the copy-paste examples. But of course this is something for @Yooooomi to decide, I just want to offer my thoughts after seeing issue after issue opened with this setting (most recent example: #403 )