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

Bugfix/546 node version abort page #570

Merged
merged 2 commits into from
May 17, 2021

Conversation

davidzwa
Copy link
Contributor

@davidzwa davidzwa commented May 13, 2021

  • Added links to octofarm.net and first hit on google for nodejs update
  • Tested by manual override
  • Tested in test Node 12 docker
  • Tested normal functioning still ok
  • Changelog

Please test and review the external links I added.

Other fixes:

  • OctoFarm port undefined bug didnt boot properly (dotenv wasnt loaded yet)
  • Load dotenv manually

@davidzwa davidzwa linked an issue May 13, 2021 that may be closed by this pull request
@davidzwa davidzwa changed the base branch from master to development May 13, 2021 08:43
@NotExpectedYet
Copy link
Member

Oooo nice! Will take a snout at this :)

Copy link
Member

@NotExpectedYet NotExpectedYet left a comment

Choose a reason for hiding this comment

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

Looking good, will test it out.

app-fallbacks.js Outdated Show resolved Hide resolved
@davidzwa davidzwa force-pushed the bugfix/546-node-version-abort-page branch 2 times, most recently from 5013042 to 53284c8 Compare May 16, 2021 18:28
@davidzwa
Copy link
Contributor Author

davidzwa commented May 16, 2021

@NotExpectedYet its easy to test this with test-node12.Dockerfile and this docker-compose entry

  octofarm-test12:
    container_name: octofarm-test12
    build:
      context: .
      dockerfile: test-node12.Dockerfile
    ports:
      - 4000:4000
    environment:
      - MONGO=....somethingfancyhere

@davidzwa davidzwa force-pushed the bugfix/546-node-version-abort-page branch from 53284c8 to 232b340 Compare May 17, 2021 08:09
Copy link
Member

@NotExpectedYet NotExpectedYet left a comment

Choose a reason for hiding this comment

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

Apart from it not loading the port this is lovely, good work man!

Gather I need to make an updating page on the OctoFarm site for this XD I clicked the link and it went no where bahaha.

@davidzwa
Copy link
Contributor Author

Apart from it not loading the port this is lovely, good work man!

Will add the dotenv config manually to prevent startup inconsistencies. I must add it cannot fully use startup checks in envUtils sadly, as it is and will not be Node 12 friendly (its so easy to make a mistake). Those checks involve writing a .env file etc, so my code will just assume default if it cant find a port setting.

Gather I need to make an updating page on the OctoFarm site for this XD I clicked the link and it went no where bahaha.

Haha I did mention it in the PR description ;)

@NotExpectedYet
Copy link
Member

Apart from it not loading the port this is lovely, good work man!

Will add the dotenv config manually to prevent startup inconsistencies. I must add it cannot fully use startup checks in envUtils sadly, as it is and will not be Node 12 friendly (its so easy to make a mistake). Those checks involve writing a .env file etc, so my code will just assume default if it cant find a port setting.

That's fine too me, if it's first boot, ie no .env then default there anyway.

Gather I need to make an updating page on the OctoFarm site for this XD I clicked the link and it went no where bahaha.

Haha I did mention it in the PR description ;)

I did read it honest XD

@davidzwa davidzwa force-pushed the bugfix/546-node-version-abort-page branch 3 times, most recently from 86bcca2 to c4034de Compare May 17, 2021 12:45
@davidzwa davidzwa force-pushed the bugfix/546-node-version-abort-page branch from c4034de to 6980eb5 Compare May 17, 2021 12:45
@davidzwa davidzwa added the fixed on dev This issue has been fixed and is on its way label May 17, 2021
Copy link
Member

@NotExpectedYet NotExpectedYet left a comment

Choose a reason for hiding this comment

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

Tested and works fine, tried with custom port and all good.

Just went to add a ticket to the website git and your already there and done baha.

Epic man nice work.

@davidzwa davidzwa merged commit f1dbeb1 into development May 17, 2021
@davidzwa davidzwa deleted the bugfix/546-node-version-abort-page branch May 17, 2021 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed on dev This issue has been fixed and is on its way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Node 13 or lower fallback view
2 participants