Remove 'suggest badges' feature #8311
Conversation
chris48s
added
frontend
core/server/server.js
Outdated
| cors: { | ||
| allowedOrigin: Joi.array().items(optionalUrl).required(), | ||
| }, |
There was a problem hiding this comment.
I've completely removed the allowedOrigin/ALLOWED_ORIGIN setting here as the 'suggest' endpoint was the only thing using it. There's a couple of things to consider here.
We could just leave this here as a setting that does nothing in case we want it again in future. I can see CORS allowed origins being a thing we could use again at some point and removing the setting then adding it in again might be kind of annoying for self-hosting users?
We could consider not removing the setting and issueing some kind of deprecation warning for a bit first. The effect of just removing the setting is that if you configure using env vars the ALLOWED_ORIGIN will just be ignored. If you have a yaml config file with allowedOrigin in it, Joi will fail to validate your config on startup. I feel like we've previously discussed how we deprecate/remove settings in the past and decided that just removing them like this is OK for that reason, but I can't find it. Anyone remember?
There was a problem hiding this comment.
I'd suggest we leave it in for the moment, and then open an issue to cover removing. I feel like that allows us to move forward, ensure we have something to keep track of the removal, and gives us more time to find out what our posture was/is on these types of things (I don't remember either 😄)
There was a problem hiding this comment.
OK cool. I've pushed a6165e7 adding that back in (plus a comment explaining)
|
|
Discoverability of the feature is actually a task of #7815 I would actually find it useful, if it worked. |
|
I think the real question should be centered around what, if any, utility this provides in its current state. The premise of badge suggestions provided some input certainly has value in concept, but I'd suggest very little of that value has actually been realized/can be realized within the current contexts. I'd be in favor of moving forward with the proposal and dropping this in its current form, and then as part of the frontend redo trying to identify a more visible and powerful way to provide the similar types of capabilities |
|
If maintaining React JS for connecting services with badges is a big burden, maybe devise static JSON format for describing badge and URL? Then different services interested in having a badge could place the description at https://example.com/badge.json And CI/CD could scrape that. |
I'm not entirely sure I follow what you are proposing, but just want to reiterate that we don't really want to use this PR as the forum to ideate on the future of badge suggestion features/implementations. From my perspective, the only conversation to be had here is whether we keep the feature as-is in its current form, or whether we remove the current implementation and feature. |
|
As I understand it, you do not want any more development, because the maintenance burden is already overwhelming, and after the redesign this feature became unusable, and need more coding, so the only viable solution for you is to kill it. That's why I proposed an alternative how it could work. I personally find the feature to automatically generating a badge given URL and service name very useful from UX experience. Than typing the badge variables by hand, but maybe it just should be a browser extension. I any case, it is your code, so I am +0.5 on removing the code that has little chances to being useful. |
|
@abitrolly - my fundamental point is that this is the wrong place to talk about alternative implementations for a feature. It's purely a binary decision, we either leave it precisely as it is or remove it. Regardless of which option we choose, we can still discuss if/how the feature can be incorporated in the new frontend. However, that future-looking discussion is best had elsewhere both because it would get lost/forgotten here, and because it has no impact on what we do relative to that aforementioned binary decision. Put differently, this code is going to get deleted/completely overhauled at some point no matter what. It's really just a question of when, and the complete overhaul isn't going to happen today, and isn't going to happen until a number of large prerequisite work items are addressed. |
|
I'm good with moving ahead with this. Given the current maintainer bandwidth I don't think we're likely to have a lively discussion, so I'd suggest we press on, especially since we can always revert should anarchy ensue (holding off on approving given the conflict) |
|
I'll sort out rebasing this at the weekend 👍 |
|
I've updated to resolve the conflicts, before we merge, any thoughts on #8311 (comment) ? |
chris48s
added
squash when passing
and removed
needs-discussion
As noted in #8275 , #8291 and probably other places over the years: The 'suggest badges' feature is somewhat hidden/not discoverable and even if you do find it, the value of the suggestions is pretty limited. We don't really maintain this feature at all as we add services (it basically makes an incomplete suggestion for github and gitlab URLs and does nothing useful for anything else) and it mainly just adds complexity to the frontend.
I propose removing this feature to clean up the frontend code a bit for ourselves and reduce confusion for users.