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 remaining Translate samples. #197

Merged
merged 1 commit into from
Sep 1, 2016
Merged

Add remaining Translate samples. #197

merged 1 commit into from
Sep 1, 2016

Conversation

jmdobry
Copy link
Member

@jmdobry jmdobry commented Aug 30, 2016

No description provided.

@@ -103,7 +104,7 @@ describe('translate:translate', function () {
var error = new Error('error');
var sample = getSample();
var callback = sinon.stub();
sample.mocks.translate.getLanguages = sinon.stub().callsArgWith(0, error);
sample.mocks.translate.getLanguages.yields(error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this should be in a separate PR - not that I care. 😛

@ace-n
Copy link
Contributor

ace-n commented Aug 31, 2016

Left a few comments. Fix those and it should be LGTM.

@jmdobry
Copy link
Member Author

jmdobry commented Aug 31, 2016

Addressed comments.

});

it('should list languages with a different target', function (done) {
program.listLanguages('es', apiKey, function (err, languages) {
assert.ifError(err);
assert(Array.isArray(languages));
assert(languages.length > 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can/should we assert that the target is recognized here? e.g.

assert.equal(languages.contains("Espanol"), true);

@ace-n
Copy link
Contributor

ace-n commented Aug 31, 2016

Just a few minor nits - fix those and I'll LGTM.

@jmdobry jmdobry force-pushed the translate branch 3 times, most recently from f840a08 to eff41c3 Compare September 1, 2016 16:34
@jmdobry
Copy link
Member Author

jmdobry commented Sep 1, 2016

Addressed comments.

@codecov-io
Copy link

Current coverage is 89.17% (diff: 100%)

Merging #197 into master will increase coverage by 0.04%

@@             master       #197   diff @@
==========================================
  Files            57         57          
  Lines          2382       2392    +10   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2123       2133    +10   
  Misses          259        259          
  Partials          0          0          

Powered by Codecov. Last update 2f9b34f...eff41c3

sinon.stub(program, 'detectLanguage');
program.main(['detect', text, '-k', apiKey]);
assert.equal(program.detectLanguage.calledOnce, true);
assert.deepEqual(program.detectLanguage.firstCall.args.slice(0, -1), [[text]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be setting an environment variable? If so, can we test that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@ace-n
Copy link
Contributor

ace-n commented Sep 1, 2016

LGTM, other than a few nits.

ace-n pushed a commit that referenced this pull request Nov 11, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 11, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 11, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 12, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 14, 2022
ace-n pushed a commit that referenced this pull request Nov 14, 2022
ace-n pushed a commit that referenced this pull request Nov 14, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 14, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 14, 2022
ace-n pushed a commit that referenced this pull request Nov 15, 2022
ace-n pushed a commit that referenced this pull request Nov 15, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 15, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
telpirion pushed a commit that referenced this pull request Nov 16, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 17, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ahrarmonsur pushed a commit that referenced this pull request Nov 17, 2022
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
jsimonweb pushed a commit to jsimonweb/nodejs-docs-samples that referenced this pull request Dec 12, 2022
kweinmeister pushed a commit that referenced this pull request Jan 11, 2023
* Fixed issues in some cases when user setting up environment.

* fix: use bash trap

Co-authored-by: Alexander Fenster <github@fenster.name>
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