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 ruby support for the default dockerfile containerizers #50

Merged
merged 3 commits into from
Oct 11, 2020
Merged

Add ruby support for the default dockerfile containerizers #50

merged 3 commits into from
Oct 11, 2020

Conversation

akhil-ghatiki
Copy link
Contributor

@akhil-ghatiki akhil-ghatiki commented Oct 11, 2020

This addresses the #26 issue.
This commit adds support for Ruby in the default dockerfile containerizers.
Followed all the required steps mentioned in the issue.

  • make generate - The commit has the generated files required.

  • make build - The build was successful.

  • move2kube translate -s srcfolder - Tested with a simple ruby application and move2kube was able to detect it successfully. (Adding the screen shot below for reference)

m2k translate

Signed-off-by: Akhil Ghatiki <akhil.ghatiki@gmail.com>
Signed-off-by: Akhil Ghatiki <akhil.ghatiki@gmail.com>
@ashokponkumar
Copy link
Member

Thanks @akhil-ghatiki for the PR. Much appreciate it. Please feel free to make more PRs as might be appropriate.

@akhil-ghatiki
Copy link
Contributor Author

akhil-ghatiki commented Oct 11, 2020

@ashokponkumar I don't exactly understand why this build failed. There are no logs for the failure in details. Can you give me any inputs on this ?

@ashokponkumar
Copy link
Member

You can check it out in https://travis-ci.com/github/konveyor/move2kube/builds/189306775. It is essentially failing because make test-style is failing. make test-style is failing because of missing license headers.

If you can add the license headers to the files you added, and did a make test-style, and if it succeeds, the build will succeed.

Signed-off-by: Akhil Ghatiki <akhil.ghatiki@gmail.com>
@codecov
Copy link

codecov bot commented Oct 11, 2020

Codecov Report

Merging #50 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #50   +/-   ##
=======================================
  Coverage   36.49%   36.49%           
=======================================
  Files          45       45           
  Lines        2343     2343           
=======================================
  Hits          855      855           
  Misses       1432     1432           
  Partials       56       56           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec483d0...91f5d6f. Read the comment docs.

@akhil-ghatiki
Copy link
Contributor Author

akhil-ghatiki commented Oct 11, 2020

You can check it out in https://travis-ci.com/github/konveyor/move2kube/builds/189306775. It is essentially failing because make test-style is failing. make test-style is failing because of missing license headers.

If you can add the license headers to the files you added, and did a make test-style, and if it succeeds, the build will succeed.

Thanks @ashokponkumar. Fixed it. Also, I think we can make test-style as part of build script. We can fail the local build if make-test fails. What's your opinion on this ?

@ashokponkumar ashokponkumar merged commit f95739d into konveyor:master Oct 11, 2020
@ashokponkumar
Copy link
Member

You can check it out in https://travis-ci.com/github/konveyor/move2kube/builds/189306775. It is essentially failing because make test-style is failing. make test-style is failing because of missing license headers.
If you can add the license headers to the files you added, and did a make test-style, and if it succeeds, the build will succeed.

Thanks @ashokponkumar. Fixed it. Also, I think we can make test-style as part of build script. We can fail the local build if make-test fails. What's your opinion on this ?

Thanks for the suggestion @akhil-ghatiki . I agree, have created a new issue #51 based on your comment.

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.

None yet

2 participants