Skip to content
This repository has been archived by the owner on Nov 24, 2021. It is now read-only.

Update enum definitions / dependencies #9

Merged
merged 1 commit into from
May 11, 2016

Conversation

sublimator
Copy link
Contributor

No description provided.

@sublimator
Copy link
Contributor Author

@shekenahglory

"istanbul": "~0.3.5",
"mocha": "~2.1.0",
"istanbul": "~0.4.3",
"mocha": "~2.4.5",
"ripple-lib": "^0.12.9"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we can remove the ripple-lib dependency now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, may as well, I don't think it's actually being used anyway

@clark800
Copy link
Contributor

clark800 commented May 9, 2016

rm -rf node_modules; npm install fails for me on OSX during the build of bufferutil and utf-8-validate. Subsequently npm test errors with Error: Cannot find module 'ripple-lib'.

@sublimator
Copy link
Contributor Author

@clark800 updated

@clark800
Copy link
Contributor

clark800 commented May 9, 2016

When I try npm linking this into ripple-lib, it causes the ripple-lib unit tests to fail. I'm not sure if it's related, but the npm link on the ripple-binary-codec fails due to lint errors, but it looks like the symlink is in place.

@sublimator
Copy link
Contributor Author

Conflicting eslint versions ?

@sublimator
Copy link
Contributor Author

Which tests fail?

It's late here. Feel free to take over this (@ anyone), or I'll look into it tomorrow.

@clark800
Copy link
Contributor

clark800 commented May 9, 2016

Which tests fail?

It doesn't even run the tests, just

Transformation error; return original code
{ [Error: Line 1: Unexpected token ILLEGAL]
  lineNumber: 1,
  description: 'Unexpected token ILLEGAL',
  index: 0 }
npm ERR! Test failed.  See above for more details.

All but the last line are normal, the illegal token doesn't usually cause a failure.

@sublimator
Copy link
Contributor Author

I also get the same message when doing an npm link as you described. I don't however when running ./node_modules/mocha/bin/_mocha without the isparta

@clark800
Copy link
Contributor

clark800 commented May 9, 2016

That's right, isparta eats error messages; here is the error I get without isparta:

      return _.indexBy(fields, 'name');},
               ^

TypeError: _.indexBy is not a function
at Function.valuesByName (index.js:120:16)

@sublimator
Copy link
Contributor Author

That should be keyBy now in lodash 4 I guess. I thought I got to that ??

@sublimator
Copy link
Contributor Author

See: douglasduteil/isparta#68 (comment)

--root src/

@sublimator
Copy link
Contributor Author

Are you by chance on the wrong commit somehow ?

@sublimator
Copy link
Contributor Author

sublimator commented May 9, 2016

Maybe we want to back off all the dependencies except the eslint/babel stuff ? ripple-lib is using lodash 3.x it seems

@clark800
Copy link
Contributor

clark800 commented May 9, 2016

I'm on dd1b64f. Ah, it looks like the problem was that npm run compile never executed because the lint check failed. I ran it manually, so now there is one failure in the unit tests: the "combine" test:

  1) RippleAPI combine:
     TypeError: Cannot read property 'from' of undefined
      at st-object.js:32:25
      at /Local/Users/clark/ripple/ripple-binary-codec/node_modules/lodash/lodash.js:13057:16
      at /Local/Users/clark/ripple/ripple-binary-codec/node_modules/lodash/lodash.js:4389:15
      at baseForOwn (/Local/Users/clark/ripple/ripple-binary-codec/node_modules/lodash/lodash.js:2652:24)
      at Function.transform (/Local/Users/clark/ripple/ripple-binary-codec/node_modules/lodash/lodash.js:13056:39)
      at Function.from (st-object.js:29:18)
      at serializeObject (binary.js:22:18)
      at Object.encode (index.js:17:21)
      at RippleAPI.combine (combine.js:34:36)
      at Context.<anonymous> (api-test.js:369:31)
      at run (node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:104:47)
      at node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:115:28
      at flush (node_modules/babel-polyfill/node_modules/core-js/modules/$.microtask.js:19:5)

@clark800
Copy link
Contributor

clark800 commented May 9, 2016

Ok, all I had to do was delete the cached eslintrc file!

@sublimator
Copy link
Contributor Author

Looks like transform in lodash 4 is getting things like toString ??

@sublimator
Copy link
Contributor Author

This "fixes" that test:

      if (typeof value === 'object') {
        return _.transform(value, (so, val, key) => {
          const field = Field.hasOwnProperty(key) && Field[key];

I'm wary of what other changes are lurking, so we should probably back lodash off to 3.x

@sublimator
Copy link
Contributor Author

Do the Connection tests always take so long ?

@sublimator
Copy link
Contributor Author

ha!

npm WARN unmet dependency /Users/ndudfield/ripple-binary-codec/node_modules/eslint requires lodash@'^4.0.0' but will load
npm WARN unmet dependency /Users/ndudfield/ripple-binary-codec/node_modules/lodash,
npm WARN unmet dependency which is version 3.10.1

@sublimator
Copy link
Contributor Author

Do we feel like updating lodash in ripple-lib ? :)

@clark800
Copy link
Contributor

clark800 commented May 9, 2016

Do we feel like updating lodash in ripple-lib ? :)

Is there any way to make it work without updating?

@sublimator
Copy link
Contributor Author

sublimator commented May 9, 2016 via email

@clark800
Copy link
Contributor

I'm wary of what other changes are lurking, so we should probably back lodash off to 3.x

I think if you just add a unit test to cover this case that caused the "combine" test to fail it should be fine to stick with 4.x and it should be good to merge

@sublimator
Copy link
Contributor Author

sublimator commented May 10, 2016 via email

@sublimator
Copy link
Contributor Author

@clark800 updated

@clark800
Copy link
Contributor

LGTM after squash

* Remove redundant `bytes` from field definitions
* Remove "date-generated" given hand editing
* Remove ripple-lib test dependency
* Fix linting errors introduced by updated eslint/eslintrc
* Ensure STObject.toJSON returns a plain object
@sublimator sublimator merged commit 11f2116 into ripple:master May 11, 2016
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.

2 participants