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

[TIMOB-26165] iOS: Fix requiring a JSON file with single quotes #10238

Merged
merged 6 commits into from Aug 9, 2018

Conversation

hansemannn
Copy link
Collaborator

JIRA: https://jira.appcelerator.org/browse/TIMOB-26165

A new file json_files/amber.json was added to the titanium-mocha-test-suite repo.

@build
Copy link
Contributor

build commented Aug 7, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

LGTM, but please avoid the ambiguous require path used in your unit test. It works, but we will eventually end support for this bad Titanium style require.

Also, you could have added the test input file into tests/Resources/jsonfiles/amber.json here in this PR and not pushed it to the suite. (Would make it easier to see together and would make it easier to cherry-pick/port to other branches).


describe('require()', function () {
it('JSON-based require() with single-quotes', function () {
var amber = require('json_files/amber');
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use a bad require style 😏

var amber = require('./json_files/amber');

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updating now. I was not sure if changes to the the tests repository will be picked up during CI build, so I placed it in the main repo instead. Good to know!

@hansemannn hansemannn merged commit 01ecbde into tidev:master Aug 9, 2018
@hansemannn hansemannn modified the milestones: 7.4.0, 7.5.0 Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants