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 package name sanitisation #212

Merged
merged 1 commit into from
Apr 10, 2018

Conversation

evanshortiss
Copy link
Contributor

Attempting to deploy an application that fails to meet openshift's
naming rules will result in misleading error messages being returned.

This commit addresses the issue by verifying the package.name is
compliant with openshift's naming restrictions.

See issue #211

@ghost ghost added the in progress label Apr 6, 2018
'openshift-rest-client': () => { return Promise.resolve({}); }
});

const tmpDir = require('os').tmpdir()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't certain about creating an item in examples/ with an invalid name, so I took this approach instead. Happy to create a file in the repo if preferable.

@coveralls
Copy link

coveralls commented Apr 6, 2018

Coverage Status

Coverage increased (+0.02%) to 96.053% when pulling 9f54b9a on evanshortiss:package-name-check into 6f8727c on bucharest-gold:master.

@lholmquist
Copy link
Member

@evanshortiss i was on PTO last week, so i'll get this today hopefully

@evanshortiss
Copy link
Contributor Author

@lholmquist no sweat, it's a minor enough issue anyways!

@@ -31,11 +31,16 @@ async function setup (options = {}) {
options.nodeshiftDirectory = '.nodeshift';
options.projectLocation = options.projectLocation || process.cwd();


Copy link
Member

Choose a reason for hiding this comment

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

looks like a stray extra line here

@lholmquist
Copy link
Member

other than that 1 comment, i think this should work.

maybe just squash those commits into 1

Attempting to deploy an application that fails to meet openshift's
naming rules will result in misleading error messages being returned.

This commit addresses the issue by verifying the package.name is
compliant with openshift's naming restrictions.

fix linter issues

remove wc file

remove added line
@evanshortiss
Copy link
Contributor Author

@lholmquist think we should be good now 👍

@lholmquist lholmquist merged commit 1c18b2a into nodeshift:master Apr 10, 2018
@ghost ghost removed the in progress label Apr 10, 2018
@lholmquist
Copy link
Member

released as 1.7.1

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

3 participants