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

Chefile handling of inner Dockerfile content #3875

Merged
merged 3 commits into from
Jan 24, 2017
Merged

Chefile handling of inner Dockerfile content #3875

merged 3 commits into from
Jan 24, 2017

Conversation

benoitf
Copy link
Contributor

@benoitf benoitf commented Jan 24, 2017

What does this PR do?

Fix #3704 by handling recipe content as a raw content and handle # for comments correctly

  1. Multiline content is escaped by default while it shouldn’t for Dockerfile content.
    Users may add “String.raw” but it’s not obvious so add it automatically

  2. Handle # comments correctly

What issues does this PR fix or reference?

Fix #3704

Changelog and Release Note Information

Changelog

Handle dockerfile content in Chefile as raw string

Release Notes:
bug only

Docs Pull Request

no doc, bug fixing.

Change-Id: I9f6c1dacbef86e2345af5d3c867620658becf8fb
Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>
Change-Id: Id5ed7cd33c1fe2907dda15918fd2ead158ab544b
Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>
@benoitf benoitf added kind/bug Outline of a bug - must adhere to the bug report template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Jan 24, 2017
@benoitf benoitf self-assigned this Jan 24, 2017
@codenvy-ci
Copy link

static keepWorkspaceRawStrings(value : string) : string {
return value.replace(/^(workspace.)(.*)=\`(.*)$/gm, '$1$2=String.raw`$3');
}

}

Choose a reason for hiding this comment

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

Newline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link

@TylerJewell TylerJewell left a comment

Choose a reason for hiding this comment

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

Approved - but there is a newline to fix.

…r comments correctly

1. Multiline content is escaped by default while it shouldn’t for Dockerfile content.
Users may add “String.raw” but it’s not obvious so add it automatically

2. Handle # comments correctly

Change-Id: Icafb2e790f83b4a62c0c4d29edf2be08cff1a96f
Signed-off-by: Florent BENOIT <fbenoit@codenvy.com>
@benoitf
Copy link
Contributor Author

benoitf commented Jan 24, 2017

@TylerJewell fixed

@benoitf benoitf merged commit b80179e into master Jan 24, 2017
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 24, 2017
@benoitf benoitf added this to the 5.2.0 milestone Jan 24, 2017
@codenvy-ci
Copy link

@JamesDrummond JamesDrummond mentioned this pull request Feb 6, 2017
9 tasks
@benoitf benoitf deleted the che#3704 branch May 24, 2017 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chefile Dockerfile Error: Fail launching agent Terminal
3 participants