rdmo icon indicating copy to clipboard operation
rdmo copied to clipboard

build(deps): dependency-updates

Open afuetterer opened this issue 1 year ago • 29 comments

This PR collects all dependency related updates for rdmo 2.2.0.

afuetterer avatar Dec 15 '23 10:12 afuetterer

👍

afuetterer avatar Mar 07 '24 15:03 afuetterer

ESlint could be updated and pinned to 8.56.0

MyPyDavid avatar Mar 08 '24 08:03 MyPyDavid

need to align

package_json_versions = {'eslint': '8.54.0', 'eslint-plugin-react': '7.34.0', 'react': '18.2.0'} pre_commit_config_versions = {'eslint': '8.54.0', 'eslint-plugin-react': '7.33.2', 'react': '18.2.0'}

MyPyDavid avatar Mar 08 '24 09:03 MyPyDavid

need to align

package_json_versions = {'eslint': '8.54.0', 'eslint-plugin-react': '7.34.0', 'react': '18.2.0'} pre_commit_config_versions = {'eslint': '8.54.0', 'eslint-plugin-react': '7.33.2', 'react': '18.2.0'}

Please feel free to delete this test, if it is annoying. I wanted to make sure, that these versions align, so I came up with this hacky test.

afuetterer avatar Mar 08 '24 09:03 afuetterer

yes thanks, it makes sense to have this but was also surprised by this hacky test ;)

MyPyDavid avatar Mar 08 '24 10:03 MyPyDavid

New setting in django-allauth, SOCIALACCOUNT_OPENID_CONNECT_URL_PREFIX with default "oidc". It will affect instances that have enabled OpenID Connect providers, the callback URLs need to be adjusted with /oidc/ if kept like this. Should we set the default SOCIALACCOUNT_OPENID_CONNECT_URL_PREFIX="" in rdmo in order to keep the URL structure the same?

https://docs.allauth.org/en/latest/release-notes/recent.html#id8.

You can now specify the URL path prefix that is used for all OpenID Connect providers using SOCIALACCOUNT_OPENID_CONNECT_URL_PREFIX. By default, it is set to "oidc", meaning, an OpenID Connect provider with provider ID foo uses /accounts/oidc/foo/login/ as its login URL. Set it to empty ("") to keep the previous URL structure (/accounts/foo/login/).

MyPyDavid avatar Mar 11 '24 14:03 MyPyDavid

Yes, thanks for catching this.

jochenklar avatar Mar 12 '24 09:03 jochenklar

Now, one of the e2e tests fail. Something wrong with the many js updates in this PR?

afuetterer avatar Mar 13 '24 06:03 afuetterer

Locally, I can't reproduce the failing e2e test. However, I think the FontAwesome icons might be missing now.

In debug browser I see the error:

downloadable font: rejected by sanitizer (font-family: "FontAwesome" style:normal weight:400 stretch:100 src index:1) source: http://localhost:8000/static/management/6d67cba9d29c7edeb12f.woff2?v=4.7.0

MyPyDavid avatar Apr 04 '24 07:04 MyPyDavid

I've added the setting for allauth > 0.60.0 SOCIALACCOUNT_OPENID_CONNECT_URL_PREFIX = ""

MyPyDavid avatar Apr 04 '24 08:04 MyPyDavid

Can we keep the node version updated as well? @jochenklar In .nvmrc it can be lts/hydrogen so that it stays on latest (v18) version.

MyPyDavid avatar Apr 04 '24 12:04 MyPyDavid

I would say we treat the node version like django or react, only update manually.

jochenklar avatar Apr 04 '24 14:04 jochenklar

the icons are still missing from the screenshot :/ https://github.com/rdmorganiser/rdmo/actions/runs/8557304508/artifacts/1385463186

MyPyDavid avatar Apr 05 '24 09:04 MyPyDavid

think I've found this icons bug, it's in the generated rdmo\management\static\management\css\management.css and the font-face paths.

@font-face {
  font-family: 'FontAwesome';
  src: url(../286b03bf4cbd3513f64f.eot?v=4.7.0);
  src: url(../286b03bf4cbd3513f64f.eot?#iefix&v=4.7.0) format('embedded-opentype'), url(../6d67cba9d29c7edeb12f.woff2?v=4.7.0) format('woff2'), url(../ec563203d3d7214eb3c8.woff?v=4.7.0) format('woff'), url(../e1b2a70250f70529242f.ttf?v=4.7.0) format('truetype'), url(../a7b5729e90ab92e4a61d.svg?v=4.7.0#fontawesomeregular) format('svg');
  font-weight: normal;
  font-style: normal;
}

the files have some sort of hashes.. when I replace that with:

@font-face {
    font-family: 'FontAwesome';
    src: url('../fonts/fontawesome-webfont.woff2?v=4.7.0') format('woff2'),
         url('../fonts/fontawesome-webfont.woff?v=4.7.0') format('woff');
    font-weight: normal;
    font-style: normal;
}

the icons appear again! 🎉

dont know exactly where these hashes are coming from.. Maybe from a css related package, that's been updated in here, like css-loader ?

PS https://stackoverflow.com/questions/68634225/webpack-5-file-loader-generates-a-copy-of-fonts-with-hash-name

MyPyDavid avatar Apr 11 '24 08:04 MyPyDavid

When should this be merged? It has already 30+ commits.

afuetterer avatar Apr 11 '24 09:04 afuetterer

yeah, think it's ready now, icons are there, maybe @jochenklar wants to look one time over all the changes?

MyPyDavid avatar Apr 11 '24 09:04 MyPyDavid

@MyPyDavid There is now a version 9.0.0 of the eslint hook. You want to include it here, last minute?

Ref: https://github.com/pre-commit/mirrors-eslint/releases/tag/v9.0.0

afuetterer avatar Apr 12 '24 07:04 afuetterer

sure thanks @afuetterer! when these ESlint versions can be aligned now ;) I'll try with reopening the closed dependency PRs?

MyPyDavid avatar Apr 12 '24 07:04 MyPyDavid

now the other npm dependencies like [email protected] don't yet support [email protected] . Now we are too much on bleeding edge with ESlint :p

I've reset the branch back to the previous last commit, ESlint version 8.54.0.

MyPyDavid avatar Apr 12 '24 08:04 MyPyDavid

Oh, I see. Meh.

afuetterer avatar Apr 12 '24 09:04 afuetterer

pre-commit needs to be updated again

Test (Python: 3.12, DB: postgres): rdmo/core/tests/test_package_status.py#L48 test_package_json_and_pre_commit_versions_match

AssertionError: assert {'eslint': '8...ct': '18.3.1'} == {'eslint': '8...ct': '18.2.0'}

Omitting 2 identical items, use -vv to show Differing items: {'react': '18.3.1'} != {'react': '18.2.0'}

Full diff: { 'eslint': '8.54.0', 'eslint-plugin-react': '7.34.0',

  • 'react': '18.2.0',
    

? ^ ^

  • 'react': '18.3.1',
    

? ^ ^ }

MyPyDavid avatar May 15 '24 09:05 MyPyDavid

do we want fix the style on these? Or just ignore those?

  • UP031: https://docs.astral.sh/ruff/rules/printf-string-formatting/

PS Was already discussed here: https://github.com/rdmorganiser/rdmo/pull/664#discussion_r1297434562 , maybe ignore for now..

Could add to pyproject.toml:

ignore = [
  ...
  "UP031",  # printf-string-formatting` 
]

rdmo/accounts/views.py:51:15: UP031 Use format specifiers instead of percent format
rdmo/core/management/commands/download_vendor_files.py:55:49: UP031 Use format specifiers instead of percent format
rdmo/core/management/commands/setup_groups.py:16:23: UP031 Use format specifiers instead of percent format
rdmo/core/utils.py:284:60: UP031 Use format specifiers instead of percent format
rdmo/core/utils.py:292:39: UP031 Use format specifiers instead of percent format
rdmo/core/utils.py:304:39: UP031 Use format specifiers instead of percent format
rdmo/core/xml.py:115:27: UP031 Use format specifiers instead of percent format
rdmo/core/xml.py:116:32: UP031 Use format specifiers instead of percent format
rdmo/management/viewsets.py:70:25: UP031 Use format specifiers instead of percent format
rdmo/management/viewsets.py:73:25: UP031 Use format specifiers instead of percent format
rdmo/management/viewsets.py:76:25: UP031 Use format specifiers instead of percent format
rdmo/options/renderers/mixins.py:51:83: UP031 Use format specifiers instead of percent format
rdmo/options/renderers/mixins.py:52:83: UP031 Use format specifiers instead of percent format
rdmo/options/renderers/mixins.py:53:88: UP031 Use format specifiers instead of percent format
rdmo/projects/exports.py:160:43: UP031 Use format specifiers instead of percent format
rdmo/projects/management/commands/prune_projects.py:28:32: UP031 Use format specifiers instead of percent format
rdmo/projects/management/commands/prune_projects.py:34:50: UP031 Use format specifiers instead of percent format
rdmo/projects/management/commands/prune_projects.py:37:27: UP031 Use format specifiers instead of percent format
rdmo/projects/tests/test_commands.py:56:9: UP031 Use format specifiers instead of percent format
rdmo/projects/tests/test_commands.py:68:20: UP031 Use format specifiers instead of percent format
rdmo/projects/views/project.py:195:27: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:17:85: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:18:84: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:47:85: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:84:47: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:86:47: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:88:47: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:150:54: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:152:54: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:154:54: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:217:51: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:219:51: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:221:51: UP031 Use format specifiers instead of percent format
rdmo/questions/renderers/mixins.py:223:51: UP031 Use format specifiers instead of percent format
rdmo/tasks/renderers/mixins.py:17:82: UP031 Use format specifiers instead of percent format
rdmo/tasks/renderers/mixins.py:18:81: UP031 Use format specifiers instead of percent format
rdmo/views/renderers/mixins.py:17:82: UP031 Use format specifiers instead of percent format
rdmo/views/renderers/mixins.py:18:81: UP031 Use format specifiers instead of percent format

MyPyDavid avatar May 22 '24 08:05 MyPyDavid

I think it should/could be fixed.

But this PR gets a little too big, don't you think? Maybe e.g. we could squash all the commits that touched the pre-commit config. 40+ commits worth of dependency-updates is a bit much.

What do you think? When should this be merged in the 2.2 branch?

afuetterer avatar May 23 '24 08:05 afuetterer

I don't mind squashing these kind of automatic updates. I don't want to squash commits by humans.

jochenklar avatar May 23 '24 08:05 jochenklar

I would like to have this branch merged first in dev-2.2.0 so that the dev-2.2.0-feature-branches can be rebased to it and can then be tested with all updates (locally or via the pipeline). Therefore, to make this branch pass the checks, I would rather make a commit here to ignore this UP031 and fix (and unignore) this style in a separate PR to the dev-2.2.0 branch.

So, ignore UP031> when checks pass > merge ?

MyPyDavid avatar May 23 '24 09:05 MyPyDavid

Sounds good to me.

afuetterer avatar May 23 '24 09:05 afuetterer

Fine with me, I guess we should enable UP031 then again and fix the issues, probably automatically. After merging the feature branches.

jochenklar avatar May 23 '24 09:05 jochenklar

ok thanks! This one looks now rebase-and-mergable to me 😅 the screenshot from the CI looks fine to me

MyPyDavid avatar May 23 '24 10:05 MyPyDavid

@jochenklar still pending for review?

  • [ ] or ok to merge?

MyPyDavid avatar May 23 '24 12:05 MyPyDavid

there is new release of rules, should I add it here? https://github.com/dfunckt/django-rules/releases/tag/v3.4.0

MyPyDavid avatar May 27 '24 10:05 MyPyDavid