Skip to content
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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Fix/log4js #5766

Merged
merged 7 commits into from
Oct 22, 2023
Merged

Fix/log4js #5766

merged 7 commits into from
Oct 22, 2023

Conversation

SamTV12345
Copy link
Member

This pull request updates log4js to the latest version. During testing I found out that if I bump typescript to 5.1.3 and npm enters legacy-peer-deps mode it somehow gets rid of all the old dependencies like request which I removed in this pull request too.

I don't know why there are some tests that fail I'll look into that later.

@SamTV12345
Copy link
Member Author

Because async await on top level is not supported. I would suggest supporting node 16>. Seems like the tests are also failing on the develop branch. So it might seems fine 😄 .

@SamTV12345 SamTV12345 mentioned this pull request Jun 26, 2023
Closed
@SamTV12345
Copy link
Member Author

I'll rebase once again when the node branch is merged into develop.

@JohnMcLear
Copy link
Member

Because async await on top level is not supported. I would suggest supporting node 16>. Seems like the tests are also failing on the develop branch. So it might seems fine 😄 .

we're now 16+

@JohnMcLear
Copy link
Member 

 npm ERR! Invalid: lock file's [email protected] does not satisfy [email protected]
npm ERR! Invalid: lock file's [email protected] does not satisfy [email protected]

I'd remake your lock file.

@JohnMcLear
Copy link
Member

This should be split up.

  1. Axios
  2. Log4JS

Also we need to talk about how this actually impacts Etherpad. Afaik this will break logging configs.

@SamTV12345
Copy link
Member Author

@JohnMcLear I seem to have hit the same problem again. All pipelines fail and I didn't succeed in just cloning the project again.

@SamTV12345
Copy link
Member Author

Dependency review is probably not happy because I had some time the latest npm in it which doesn't work out of the box. If that pull request will crash instances I would postpone this pull request to milestone 2.0 @JohnMcLear

@JohnMcLear
Copy link
Member

Split it down to minor changes IE axios and see what works/doesn't then go from there : )

@SamTV12345
Copy link
Member Author

Axios has been merged in #5776 . This contains only the log4js changes necessary to migrate to v6

@github-actions github-actions bot added the Stale No recent activity label Sep 4, 2023
@SamTV12345 SamTV12345 removed the Stale No recent activity label Oct 22, 2023
@SamTV12345 SamTV12345 merged commit f649b1e into develop Oct 22, 2023
35 of 37 checks passed
@SamTV12345 SamTV12345 deleted the fix/log4js branch October 22, 2023 16:26
boutsak pushed a commit to weaveeducation/etherpad-lite-legacy that referenced this pull request Jan 5, 2024
@SamTV12345 @boutsak
Fix/log4js (ether#5766)
7d8b7a3 
* Updated log4js

* Updated log4js version.

* Removed requests and moved to axios.

* Fixed startup.

* Fixed windows dependency install.

* Fixed import export rate limiter.

* Fixed all log levels.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SamTV12345 @JohnMcLear