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

throw error if bundleConfig does not exist #6578

Merged
merged 4 commits into from Dec 25, 2017
Merged

Conversation

yairans
Copy link
Contributor

@yairans yairans commented Dec 24, 2017

FEC-7654

@yairans yairans self-assigned this Dec 24, 2017
@yairans yairans requested a review from OrenMe December 24, 2017 12:56
@@ -428,6 +426,9 @@ private function initMembers()

$this->bundleConfig = json_decode($confVars, true);
$this->mergeVersionsParamIntoConfig();
if (!$this->bundleConfig) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

mergeVersionsParamIntoConfig uses the bundle config ($this->bundleConfig = array_merge($this->bundleConfig, $versionsArr);) so you may want to move this check up.

If bundleConfig in null and you will call array_merge it will result in a warning.

Another option is to define the bundleConfig to default value of an empty array and change this check accordingly.

$isLatestVersionRequired = array_search("{latest}", $this->bundleConfig) !== false;
$isBetaVersionRequired = array_search("{beta}", $this->bundleConfig) !== false;
//if latest/beta version required set version number in config obj
$isLatestVersionRequired = array_search("{latest}", $this->bundleConfig) !== false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move the strings "{latest}" and "{beta}" to be class constants since you use them more than once.

@yairans yairans merged commit ca3e4b2 into Mercury-13.10.0 Dec 25, 2017
@yairans yairans deleted the validate-bundle branch December 25, 2017 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants