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

TKNECO-20: Import s2i-dotnet Task #35

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

Aneesh-M-Bhat
Copy link
Contributor

@Aneesh-M-Bhat Aneesh-M-Bhat commented Aug 8, 2023

Story: https://issues.redhat.com/browse/TKNECO-20

Progress:
Currently, I have updated the script itself for adding .s2i/environment file to the source code which will allow us to add the required DOTNET_STARTUP_PROJECT env variable, that helps with showing .csproj file to be published for .NET

PTAL, we could add a volume mount for .s2i/environment or could copy some file given by user to the said path or keep the same implementation, need feedback on this matter.

To run the test, use make test-e2e-s2i-dotnet E2E_S2I_PARAMS_REVISION=main

@Aneesh-M-Bhat
Copy link
Contributor Author

Updated the PR, now we are using a parameter in the Makefile as well as the task, for the tests we use the parameter E2E_S2I_PARAMS_ENV_VARS in the Makefile, the task has also been updated appropriately to make use of ENV_VARS param, which will then be used to make an environment file, this will be used as the environment file for the s2i build command

Copy link
Contributor

@otaviof otaviof left a comment

Choose a reason for hiding this comment

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

Overall it looks good, adding some more changes needed.

scripts/s2i-generate.sh Outdated Show resolved Hide resolved
scripts/s2i-generate.sh Outdated Show resolved Hide resolved
scripts/s2i-generate.sh Outdated Show resolved Hide resolved
scripts/s2i-generate.sh Outdated Show resolved Hide resolved
templates/spec-s2i.tpl Show resolved Hide resolved
templates/spec-s2i.tpl Outdated Show resolved Hide resolved
templates/spec-s2i.tpl Outdated Show resolved Hide resolved
Copy link
Contributor

@otaviof otaviof left a comment

Choose a reason for hiding this comment

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

We're almost there, just a few minor comments but looks quite good already 👍

scripts/s2i-generate.sh Outdated Show resolved Hide resolved
scripts/s2i-generate.sh Outdated Show resolved Hide resolved
scripts/s2i-generate.sh Outdated Show resolved Hide resolved
scripts/s2i-generate.sh Outdated Show resolved Hide resolved
scripts/s2i-generate.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@otaviof otaviof left a comment

Choose a reason for hiding this comment

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

/lgtm 👏

@otaviof otaviof merged commit f55980e into openshift-pipelines:main Aug 25, 2023
1 check passed
@otaviof
Copy link
Contributor

otaviof commented Aug 29, 2023

Fixes #19.

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