rubygems / rubygems.org

The Ruby community's gem hosting service.
https://rubygems.org
MIT License
2.31k stars 916 forks source link

Use summary instead of description in gems list #3439

Closed svoop closed 1 year ago

svoop commented 1 year ago

Given the "bridgetown_credentials" gem as an example:

# bridgetown_credentials.gemspec
(...)
spec.summary     = "Rails-like encrypted credentials for Bridgetown"
spec.description = <<~END
  Credentials like passwords, access tokens and other secrets are often passed
  to sites each by it's own ENV variable. This is both uncool, non-atomic and
  therefore unreliable. Use this plugin to store your credentials in encrypted
  YAML files which you can safely commit to your source code repository. In
  order to use all of them in Bridgetown, you have to set or pass exactly one
  ENV variable holding the key to decrypt.
END

On the gem details page, the "description" is printed – which is totally fine:

image

However, when searching gems, the result list also uses the "description" and crops it after some 80 chars:

image

What do you think, wouldn't it be better to print the "summary" instead and use the cropped "description" only as a fallback?

stirlhoss commented 1 year ago

I agree with this. A lot of times the first few words of the description don't actually tell the user much about what the gem does, as in the example above. Seems like an easy fix and it would really improve functionality.

simi commented 1 year ago

OK, let's try this. Anyone able to open PR? We can keep it on the UI side only for now (not sure if API could be even affected).

stirlhoss commented 1 year ago

I am looking at it but I am new to the codebase so it might take me longer than it should. If no one else has claimed it by the time I figure it out I will open one.

A bit new to this so I am going to ask some potentially silly questions.

Sorry for the extra questions, just want to make sure that I do things in the right way/order.

svoop commented 1 year ago

Thanks for having a go, @stirlhoss. The actual rendering happens in this partial. It ultimately uses this helper which is hardcoded to the description. You can change this behavior in these two view files only – which is probably what @simi ment with "UI only". (Of course, there might be some tests as well.)

Just ping me if you need any help!

simi commented 1 year ago
  • Is it normal to open a PR for a change even if it isn't done yet?

feel free to open draft PR

  • When you say keep it on the UI side are you just referring to this being a change on the Front-end of the application? ie changes to the MVC?

Yup, as @svoop mentioned, changes in some view templates would be needed.

stirlhoss commented 1 year ago

@svoop Thanks for the input. These were the files that I had identified so good to know I was on the right track.

Not sure how late into the week it will take me. I am fumbling around a bit with getting the environment set up but after that is ironed out I think it will be an easy change.

stirlhoss commented 1 year ago

PR is up. It might be an overly simple solution that needs doesn't cover every base. If that is the case I am happy to keep working on it.

I added a unit test to make sure it is functioning and it seems to be without any issues. All tests passed with this change in place. Looking forward to getting some feedback on this.

ayinloya commented 1 year ago

This seems to have been merged, is there any pending work? @simi @stirlhoss

stirlhoss commented 1 year ago

This seems to have been merged, is there any pending work? @simi @stirlhoss

Nope, this is taken care of and closed.

svoop commented 1 year ago

@stirlhoss Thanks a lot!!