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

[PR] Upgrading to Node16 and temporarily disabling Review App and API Definition tests #405

Merged
merged 14 commits into from
Aug 15, 2023

Conversation

LuchoTurtle
Copy link
Member

@LuchoTurtle LuchoTurtle commented Aug 14, 2023

closes #404
closes #406

Additionally, it upgrades the mix.lock file, thus fixing the compilation error when running flyctl (related to the useful package).

This also temporarily disabled the Review App and API Definition Github Action pipelines due to #407

@LuchoTurtle LuchoTurtle added in-progress An issue or pull request that is being worked on by the assigned person chore a tedious but necessary task often paying technical debt labels Aug 14, 2023
@LuchoTurtle LuchoTurtle self-assigned this Aug 14, 2023
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #405 (4cc270a) into main (957efcc) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #405   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          490       490           
=========================================
  Hits           490       490           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@LuchoTurtle
Copy link
Member Author

I've seen this error popped up ever since useful was upgraded. I've tried circumventing it and it appears to have been related with the release that occurred 4 days ago.

Error: ** (CompileError) lib/useful.ex:24: Plug.Upload.__struct__/0 is undefined, cannot expand struct Plug.Upload. Make sure the struct name is correct. If the struct name exists and is correct but it still cannot be found, you likely have cyclic module usage in your code

I'm opening an issue on this. Easy route would be to revert this upgrade but I wager we'll have to check what's going on in https://github.com/dwyl/useful.

@nelsonic
Copy link
Member

I find it very peculiar that we're getting this Plug.Upload undefined error when Plug is in the mix.lock file:

mvp/mix.lock

Line 56 in 4c0639a

"plug": {:hex, :plug, "1.14.2", "cff7d4ec45b4ae176a227acd94a7ab536d9b37b942c8e8fa6dfc0fff98ff4d80", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "842fc50187e13cf4ac3b253d47d9474ed6c296a8732752835ce4a86acdf68d13"},

Plug.Upload is just a module in: https://github.com/elixir-plug/plug/blob/v1.14.2/lib/plug/upload.ex#L5

@nelsonic
Copy link
Member

Review App Script failed: https://github.com/dwyl/mvp/actions/runs/5864562055/job/15899841640?pr=405 🤷‍♂️
image

Re-running ... ♻️

@nelsonic
Copy link
Member

Review App fails: #407

@LuchoTurtle LuchoTurtle changed the title [PR] Upgrading to Node16 [PR] Upgrading to Node16 and fixing useful bug Aug 15, 2023
@LuchoTurtle LuchoTurtle changed the title [PR] Upgrading to Node16 and fixing useful bug [PR] Upgrading to Node16 and fixing Review App GH action not deploying with flyctl Aug 15, 2023
@nelsonic
Copy link
Member

@LuchoTurtle given that the build is passing (with Review Apps disabled) feel free to assign for review + merge.
As noted in #407 I think that the deploy from GitHub in the Review Apps script issue is out of our control ...
If other people are having the same issue: #407 (comment)
We should not spend any more time on this today.
Rather we should check back on the Fly.io forum in a few days time and see if anyone has figured it out. 🤞

@LuchoTurtle LuchoTurtle changed the title [PR] Upgrading to Node16 and fixing Review App GH action not deploying with flyctl [PR] Upgrading to Node16 and temporarily disabling Review App and API Definition tests Aug 15, 2023
@LuchoTurtle LuchoTurtle marked this pull request as ready for review August 15, 2023 09:15
@LuchoTurtle
Copy link
Member Author

@nelsonic this should be good to go. 👌

@LuchoTurtle LuchoTurtle removed the chore a tedious but necessary task often paying technical debt label Aug 15, 2023
@LuchoTurtle LuchoTurtle added the awaiting-review An issue or pull request that needs to be reviewed label Aug 15, 2023
@LuchoTurtle LuchoTurtle assigned nelsonic and unassigned LuchoTurtle Aug 15, 2023
Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LuchoTurtle looks good. thanks. 🙏

@nelsonic nelsonic merged commit d919ce3 into main Aug 15, 2023
@nelsonic nelsonic deleted the node_deprecated-#404 branch August 15, 2023 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed in-progress An issue or pull request that is being worked on by the assigned person
Projects
None yet
2 participants