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

add images for nodejs version 14 #246

Merged
merged 9 commits into from
Jul 27, 2020
Merged

add images for nodejs version 14 #246

merged 9 commits into from
Jul 27, 2020

Conversation

pkubatrh
Copy link
Member

No description provided.

@pkubatrh pkubatrh force-pushed the node_14 branch 3 times, most recently from 23d5766 to 81b0697 Compare July 20, 2020 07:31
as Fedora 30 is already EOL and images got removed
@phracek phracek self-requested a review July 20, 2020 12:53
Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

Thanks for this new version and this PR!. Some small comments and improvements.

14/Dockerfile.fedora Outdated Show resolved Hide resolved
14/Dockerfile.fedora Outdated Show resolved Hide resolved
14/s2i/bin/usage Outdated Show resolved Hide resolved
14/s2i/bin/usage Outdated Show resolved Hide resolved
@pkubatrh
Copy link
Member Author

Seems like we have a limitation of only being able to have one image version per Fedora release, else the higher version overrides the lower one :/

14:36:41 -> Tagging image 'e1159a781b9e' as 'f32/nodejs:10' and 'f32/nodejs:latest' and 'f32/nodejs:20200720-d63b187'
14:36:41 -> Tagging image '760e0beaa931' as 'f32/nodejs:12' and 'f32/nodejs:latest' and 'f32/nodejs:20200720-d63b187'
14:36:41 -> Tagging image 'e4753c60f782' as 'f33/nodejs:14' and 'f33/nodejs:latest' and 'f33/nodejs:20200720-d63b187'

@pkubatrh
Copy link
Member Author

New changes pushed. Will create a tracker for the issue mentioned above and downgrade node 10 to fc31 base image for now.

14/s2i/bin/assemble Outdated Show resolved Hide resolved
Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

Just one comment, regarding nodemon. It should be a part of Dockerfile.fedora.

What about imagestreams? Do you plan to updated as well? If not, then I think, tests are going to be failed on test_lastest_imagestreams?

If imagestreams 14 do not exist yet, then I think Makefile should have a VERSIONS like:

VERSIONS=10 14 12

What do you think?

@phracek
Copy link
Member

phracek commented Jul 27, 2020

Sorry for noise regarding imagestreams. If .exclude-{OS} are present, thank tests is skipped.
Then only nodemon issue.

Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!. LGTM as soon as tests are passing!!!!

Fedora 31 does not have nodejs-nodemon so no changes for node 10
@pkubatrh
Copy link
Member Author

[test]

@phracek phracek merged commit bf34728 into sclorg:master Jul 27, 2020
@pkubatrh pkubatrh deleted the node_14 branch July 28, 2020 08:13
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