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

Issue #105 #185

Merged
merged 7 commits into from
Feb 20, 2024
Merged

Issue #105 #185

merged 7 commits into from
Feb 20, 2024

Conversation

Srishti-j18
Copy link
Contributor

fixed the issue dotnet sample app needs a default entrypoint .

@Srishti-j18 Srishti-j18 requested a review from a team as a code owner February 1, 2024 21:56
@@ -8,6 +8,7 @@ set layers_dir=%1
echo [[processes]] >> %layers_dir%\launch.toml
echo type = "web" >> %layers_dir%\launch.toml
echo command = ["app.bat"] >> %layers_dir%\launch.toml
default = true
Copy link
Member

Choose a reason for hiding this comment

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

@Srishti-j18 you need to write the value default = true into the launch.toml file, just follow the same way line 10 with the echo command

Copy link
Member

Choose a reason for hiding this comment

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

You also need to do the equivalent for the bash buildpack

Copy link
Member

@jjbustamante jjbustamante left a comment

Choose a reason for hiding this comment

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

Hi @Srishti-j18,

Thanks a lot for the PR! you still need to do some changes to make it work! but it is ok

@@ -8,6 +8,7 @@ set layers_dir=%1
echo [[processes]] >> %layers_dir%\launch.toml
echo type = "web" >> %layers_dir%\launch.toml
echo command = ["app.bat"] >> %layers_dir%\launch.toml
default = true
Copy link
Member

Choose a reason for hiding this comment

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

You also need to do the equivalent for the bash buildpack

@Srishti-j18
Copy link
Contributor Author

Srishti-j18 commented Feb 2, 2024

Oh, my bad. I'll make changes according to your instructions.

@Srishti-j18 Srishti-j18 closed this Feb 2, 2024
@Srishti-j18 Srishti-j18 reopened this Feb 2, 2024
Signed-off-by: Srishti-j18 <[email protected]>
@@ -8,6 +8,7 @@ set layers_dir=%1
echo [[processes]] >> %layers_dir%\launch.toml
echo type = "web" >> %layers_dir%\launch.toml
echo command = ["app.bat"] >> %layers_dir%\launch.toml
echo default = "true" >> %layers_dir%\launch.toml
Copy link
Member

Choose a reason for hiding this comment

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

Should the true be quoted or unquoted here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops sorry, It should be unquoted here, because, In TOML syntax, boolean values are represented without quotes to distinguish them from string values...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AidanDelaney ,I have corrected this typo..
Actually I corrected the format for Linux previously but forgot to update it for Windows.

AidanDelaney and others added 3 commits February 5, 2024 22:33
Use the latest version of the buildpacks API

Signed-off-by: Aidan Delaney <[email protected]>
Signed-off-by: Srishti-j18 <[email protected]>
Using linux/amd64 allows linux builds on emulators (eg. macos
M1/2/3).  However it breaks Windows builds

Signed-off-by: Aidan Delaney <[email protected]>
Signed-off-by: Srishti-j18 <[email protected]>
Copy link
Member

@jjbustamante jjbustamante left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@AidanDelaney AidanDelaney left a comment

Choose a reason for hiding this comment

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

This is great. Thanks a lot for your attention to the detail.

@AidanDelaney AidanDelaney merged commit 70d5d8d into buildpacks:main Feb 20, 2024
4 checks passed
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