Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Bump version of jQuery to 3.6.4 & updated ref links#8909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
auvipy merged 1 commit intoencode:masterfromauvipy:jq
Mar 28, 2023

Conversation

@auvipy
Copy link
Collaborator

No description provided.

@auvipyauvipy requested a review froma teamMarch 16, 2023 11:05
@baseplate-admin
Copy link
Contributor

Hey forgive me for asking but can't we link againstdjango admin's jQuery?

@auvipy
Copy link
CollaboratorAuthor

Hey forgive me for asking but can't we link againstdjango admin's jQuery?

that's a good question! DRF uses twitter bootstrap which django admin doesn't. I followed the historical approach here. But if old maintainer of the project suggest for using django admins jquery I will update accordingly. another point to consider here is that, the latest version of twbs has stopped using jquery & switched to vanilla JS. as DRF will also upgrade it's twbs version, it most likely will drop usage of jquery

baseplate-admin reacted with thumbs up emoji

@auvipyauvipy marked this pull request as ready for reviewMarch 24, 2023 06:43
@auvipy
Copy link
CollaboratorAuthor

@tschwaerzl did you notice any regression in existing UI's??

@baseplate-admin
Copy link
Contributor

baseplate-admin commentedMar 25, 2023
edited
Loading

But if old maintainer of the project suggest for using django admins jquery I will update accordingly

This is a good answer. Thanks :D


Should i adress this in a Issue that can get the maintainers attention or should i ping here?

@auvipy
Copy link
CollaboratorAuthor

I am also a maintainer now for your kind info :)

@kevin-brown
Copy link
Contributor

kevin-brown commentedMar 25, 2023
edited
Loading

I'm fairly certain that Django is also in the process of dropping the dependency on jQuery so I don't think it makes sense to switch over to that one at this time. I'm fine updating our vendored version to a newer one, but we should also look towards removing the dependency in the future.

I'm pretty sure we'd also have to take a new dependency against Django's admin, which I don't think we otherwise require.

auvipy and baseplate-admin reacted with thumbs up emoji

@auvipy
Copy link
CollaboratorAuthor

I think we should only focus on DRF vendroed version for now as this fix some security / other bugs. and for future, moving away from jquery

baseplate-admin reacted with thumbs up emoji

@baseplate-admin
Copy link
Contributor

baseplate-admin commentedMar 28, 2023
edited
Loading

I'm fairly certain that Django is also in the process of dropping the dependency on jQuery so I don't think it makes sense to switch over to that one at this time.

I am fairly certain thats not happening The reasoning is that too many dependencies require jQuery to be present in django admin ( eg:django-hstore-fields)

I think we should only focus on DRF vendroed version for now as this fix some security / other bugs. and for future, moving away from jquery

Fair enough. I respect your opinion.

@tschwaerzl did you notice any regression in existing UI's??

If my opinion matters. No i didn't find any regression during my initial ( albeit very light testing )

I am also a maintainer now for your kind info :)

Apologies. I was not aware of this. Forgive my rude response from before. ( Congratulations !! )


Since django also updated their jquery to 3.6.4 i think we can move with this.

@thomagos
Copy link

No issues found. Everything working as it should be.

@auvipy
Copy link
CollaboratorAuthor

Apologies. I was not aware of this. Forgive my rude response from before. ( Congratulations !! )

never mind my comment was purely pun intended! no offense taken! you views are highly appreciated. the only reasoning here against using django jquery is, DRF is based on twbs. that's the issue. If you guys can confirm no regression happening and also check the version file in this PR is security checked then I can merge this.

@auvipyauvipy added this to the3.15 milestoneMar 28, 2023
@auvipyauvipy merged commit29b6dd8 intoencode:masterMar 28, 2023
@auvipyauvipy deleted the jq branchMarch 28, 2023 09:43
@auvipy
Copy link
CollaboratorAuthor

we have another followup PR#9094

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

2 more reviewers

@pauloxnetpauloxnetpauloxnet approved these changes

@thomagosthomagosthomagos approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.15

Development

Successfully merging this pull request may close these issues.

5 participants

@auvipy@baseplate-admin@kevin-brown@thomagos@pauloxnet

[8]ページ先頭

©2009-2025 Movatter.jp