Skip to content
This repository has been archived by the owner on Feb 28, 2020. It is now read-only.

feat: refactor to use bluemix object to specify services #372

Merged
merged 2 commits into from
Nov 9, 2017

Conversation

EnriqueL8
Copy link
Contributor

@EnriqueL8 EnriqueL8 commented Nov 7, 2017

Two dependencies of the project (generator-ibm-service-enablement and generator-ibm-cloud-enablement) use a different "bluemix" object to specify the cloud properties than we were using previously in this generator.

When we shifted to using these dependencies for generating cloud related content we kept our original format for specifying services intact to reduce the churn and risk associated with the refactor.

The time has come to address that technical debt and update the generators to use the bluemix object everywhere and remove the legacy format and associated conversion code.

@EnriqueL8 EnriqueL8 self-assigned this Nov 7, 2017
@EnriqueL8 EnriqueL8 requested a review from tunniclm November 7, 2017 13:59
app/index.js Outdated
@@ -61,11 +61,42 @@ module.exports = Generator.extend({
})
},

_isTrue: function (value) {
return (value === true || value === 'true')
},
Copy link
Contributor

Choose a reason for hiding this comment

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

make this a local function, we only use it in initSpec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

app/index.js Outdated
@@ -490,7 +532,7 @@ module.exports = Generator.extend({
// NOTE(tunniclm): no need to do anything for memory it is the default
// if no crudservice is passed to the refresh generator
if (answer.store === 'Cloudant') {
this._addService('cloudant', { name: 'crudDataStore' })
this._addService('cloudant', 'crudDataStore', true)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove , true it is ignored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

app/index.js Outdated
})
},

promptConfigureWatsonConversation: function () {
if (this.skipPrompting) return
if (!this.servicesToConfigure) return
if (!this.servicesToConfigure.watsonconversation) return

this.log()
Copy link
Contributor

Choose a reason for hiding this comment

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

please add this log back for consistency of presentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

test/unit/app.js Outdated
@@ -1277,7 +1270,7 @@ describe('Unit tests for swiftserver:app', function () {
appType: 'scaffold',
appName: applicationName,
capabilities: [],
services: { objectstorage: {} }
services: { objectStorage: {} }
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't actually test the key objectStorage exists unless we put something real in the value (not the empty object)

Boilerplate code for creating a client object for the Kitura-redis API is included inside `Sources/Application/Application.swift` as an `internal` variable available for use anywhere in the `Application` module.

The connection details for this client are loaded by the [configuration](#configuration) code and stored in a `struct` for easy access when creating connections to Redis.
<% } -%>
<% } -%>

### Configuration
Your application configuration information is stored in the `config.json` in the project root directory. This file is in the `.gitignore` to prevent sensitive information from being stored in git.
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole section is out of date and we should remember to create an issue to fix it.

test/unit/app.js Outdated
@@ -1427,7 +1420,7 @@ describe('Unit tests for swiftserver:app', function () {
appType: 'scaffold',
appName: applicationName,
capabilities: [],
services: { watsonconversation: {} }
services: { conversation: {} }
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't actually test the key conversation exists unless we put something real in the value (not the empty object)

test/unit/app.js Outdated
@@ -1501,7 +1494,7 @@ describe('Unit tests for swiftserver:app', function () {
appType: 'scaffold',
appName: applicationName,
capabilities: [],
services: { alertnotification: {} }
services: { alertNotification: {} }
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't actually test the key alertNotification exists unless we put something real in the value (not the empty object)

objectstorage: [{}]
bluemix: {
backendPlatform: 'SWIFT',
objectStorage: [{ serviceInfo: {} }]
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't actually test the key serviceInfo exists unless we put something real in the value (not the empty object)

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if this checks whether objectStorage exists or not...

@codecov-io
Copy link

codecov-io commented Nov 8, 2017

Codecov Report

Merging #372 into develop will decrease coverage by 0.4%.
The diff coverage is 91.03%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #372      +/-   ##
===========================================
- Coverage    89.66%   89.26%   -0.41%     
===========================================
  Files            9        9              
  Lines         1268     1276       +8     
===========================================
+ Hits          1137     1139       +2     
- Misses         131      137       +6
Impacted Files Coverage Δ
refresh/index.js 92.89% <100%> (+1.05%) ⬆️
app/index.js 88.31% <82.35%> (-2.49%) ⬇️
lib/helpers.js 93.08% <97.29%> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58e3348...134ebd1. Read the comment docs.

@tunniclm tunniclm merged commit ebe6149 into develop Nov 9, 2017
@tunniclm tunniclm deleted the use_bluemix_object branch November 9, 2017 16:11
@tunniclm tunniclm changed the title feat: Removing Swiftserver Devex feat: refactor to use bluemix object to specify services Nov 9, 2017
tunniclm added a commit that referenced this pull request Dec 7, 2017
Release 4.2.0

### Bug Fixes

* **package:** update generator-ibm-service-enablement to version 0.1.0 ([2948e7d](2948e7d)), closes [#371](#371)
* add url to Push Notifications service ([#379](#379)) ([9660ec7](9660ec7))
* PR title and message are properly generated ([#382](#382)) ([f2ba505](f2ba505))
* **package:** update generator-ibm-service-enablement to version 0.6.1 ([#391](#391)) ([a0bd2e5](a0bd2e5))


### Features

* refactor to use bluemix object to specify services ([#372](#372)) ([ebe6149](ebe6149))
@tunniclm tunniclm added this to the 4.2.0 milestone Dec 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants