#9 Bootstrap 5 template rewrite + In-page JavaScript searching + Revised template logic
Opened by t0xic0der. Modified
t0xic0der/fedora-gather-easyfix experience-modernization  into  master

Download 9.patch

This template rewrite brings the following things to the table.

  • Based on Bootstrap 5 CSS framework
  • Out-of-the-box responsiveness for variable viewport sizes
  • In-page searching with respect to project names and issue titles
  • Elegant handling of issue status, type and component
  • Including assets from inside the local storage to avoid CORS
  • Fixed navbar links and removed redundant links

Do take a look and let me know if there are anymore changes needed in here.

Thanks.

Should this be translatable content ? Currently missing those tags.

Hey @copperi, the current rendition of easyfix does not have them so I am not really sure if this should have this or not. (Maybe it should? You would want to ask around for more details on this.)

Thanks for the review.

I dislike the habit that pinpoints the dependencies to a very very specific version. Please enforce a minimal version if needed but let's not specify the higher version, unless we know that a newer version breaks the app.

we can drop fedmenu, it's being deprecated

A lot of the dependencies listed here don't seem to be used by the project: certifi, chardet, idna, requests, requests-oauthlib, oauthlib, urllib3, six

Could you check it?

I dislike the habit that pinpoints the dependencies to a very very specific version. Please enforce a minimal version if needed but let's not specify the higher version, unless we know that a newer version breaks the app.

Yep, I ran a pip3 freeze > requirements.txt cold. Let me remove the version peculiarities in it.

we can drop fedmenu, it's being deprecated

Alright. Will remove it.

A lot of the dependencies listed here don't seem to be used by the project: certifi, chardet, idna, requests, requests-oauthlib, oauthlib, urllib3, six

Could you check it?

Okay, I will reflect the changes in the newer requirements.txt.

So,

  1. python-bugzilla uses requests and requests in turn uses certifi, chardet, idna and urllib3.
  2. mwclient uses six and requests-oauthlib and requests-oauthlib requires idna, certifi, urllib3, chardet and oauthlib.

There were no differences possible in the requirements.txt hence in terms of dependency listing but I provided for given version or above check for dependencies.

Re-requesting review @pingou.

  1. python-bugzilla uses requests and requests in turn uses certifi, chardet, idna and urllib3.

So the first one is required in the file, the others not.
Imagine python-bugzilla drops requests, or requests drops certifi, if you keep
it in the requirements.txt you will still bring it while it's not needed.

  1. mwclient uses six and requests-oauthlib and requests-oauthlib requires idna, certifi, urllib3, chardet and oauthlib.

Same as above.

You put your requirements in that file, not the dependencies of your
dependencies, unless there is something optional or a bug in the dependency
chain higher up.

Understood. I'd change them right away.

Amended 6219dcf https://pagure.io/fork/t0xic0der/fedora-gather-easyfix/c/6219dcfe3e5f89e2da65c0688ecac45b5e2b5dc9 to include only the necessary dependencies.

@pingou The suggested changes have been made and this PR is ready to be reviewed again. Please do take a look whenever you get time.

Had just a quick glance, is there any reason for renaming the template?

Also, do we want to review the git commit history?

Not really. Should I fall back to the original name?

Not really. Should I fall back to the original name?

One less change, always good :)

Alright. I will revert the renaming.

Is there anything else to correct/change so that I can do them all in a go?

Also, do we want to review the git commit history?

Of the easyfix repo? or of those projects listed there?

Also, do we want to review the git commit history?

Of the easyfix repo? or of those projects listed there?

Of the PR here

I quickly sifted through the commit history and found that nearly all the histories have meaningful commit messages and there are notable differences between subsequent commits that there would not be any need of rewriting the history (if that's what you're asking).

@pingou, I have made some more changes. I have reverted back to template.html name and made corrections to setup.py regarding the static file inclusion. Should we bump its version when the modernization is complete?

Also, I have included the logo provided by @duffy here https://pagure.io/design/issue/714#comment-717787 and have accounted for the suggestions made by @duffy and @mleonova about the look and feel here https://pagure.io/design/issue/715#comment-717513 and here https://pagure.io/design/issue/715#comment-716852.

The modernization would be further discussed about by the design team as a heuristic evaluation of the progress made so far was done. There's more about it here https://pagure.io/design/issue/715#comment-718574.

Re-requesting review.

Should we include the previous people involved? Or maybe just drop that line?

rebased onto 66a2c839dc024f8a9af3ff38952558b417bc51bd

To https://pagure.io/fedora-gather-easyfix/pull-request/9#comment-146146, that was automatically generated by my IDE but I believe that we would want to add the names of the previous people involved there instead of removing that line.

The design is worked upon here in this ticket https://pagure.io/design/issue/715 so I would make changes once the mockups have been decided upon.

To https://pagure.io/fedora-gather-easyfix/pull-request/9#comment-146146, that was automatically generated by my IDE but I believe that we would want to add the names of the previous people involved there instead of removing that line.

Dropping it sounds easier to me but it's up to you

Metadata