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.

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

update axios version fixes #104 #4

Merged
merged 1 commit into from
Jan 19, 2023
Merged

update axios version fixes #104 #4

merged 1 commit into from
Jan 19, 2023

Conversation

boerniee
Copy link

Hello,

I have updated the axios version from 0.27.0 to 0.27.2.
The older version had a bug which doesnt set the Content-Type correctly to multipart-form with boundary data while uploading a quick entry image.

This pull request addresses the issue akhilrex#104.

BR

@AlfHou
Copy link
Owner

AlfHou commented Jan 18, 2023

Thank you for the PR @boerniee! I think the reason it is failing is some error in the Test UI CI pipeline. @djjudas21 you were the one who added them right? Do you mind taking a look at fixing it? It might be as easy as just adding a npm install step.

@djjudas21
Copy link
Collaborator

Yes I added the tests, although they were boilerplate ones I copied from somewhere. I'll have a look this afternoon and see if I can get the pipeline passing 😄

@djjudas21
Copy link
Collaborator

I've been looking at the output of the failed tests but it looks like something is actually wrong with the code. I have no idea what I'm doing with Node but it looks like some kind of error running a backend maybe? I guess you can't test the UI in isolation from the backend

Error: Error: connect ECONNREFUSED 127.0.0.1:3000

https://github.com/AlfHou/hammond/actions/runs/3554879799/jobs/5971242698

@AlfHou
Copy link
Owner

AlfHou commented Jan 19, 2023

The ^ in the package file should already include the .2 version of axios: StackOverflow. I think the problem here is that the docker image on akhilrex's docker hub hasn't been updated in a while, and is still stuck on the .0 version.
However, I don't see any problem with specifying this version in the package file either, so I'll merge it.

I think the build errors we are experiencing in the CI is unrelated to the change in this PR. However we should probably look at getting them fixed. I'll open an issue for it and take a closer look when I have the time (probably next week).

@AlfHou AlfHou merged commit 5aabeda into AlfHou:master Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants