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

Rename src directory to lib #3338

Merged
merged 3 commits into from Jul 10, 2019

Conversation

ricardograca
Copy link
Member

This restores backwards compatibility with versions before 0.18 if you were importing sub-packages directly, e.g. lib/query/builder. This technique is used when mocking certain parts of the Knex API for testing other applications that use Knex.

Link to discussion: bookshelf/bookshelf#1988

Note that there are no drawbacks to this change, other than breaking backwards compatibility again for people that could have used the same technique for versions 0.18.0-0.18.3, but considering this was released a couple of weeks ago this scenario is unlikely.

The idea here is to maintain the internal structure of the files available in the released build as in previous versions.

- This restores backwards compatibility with versions before 0.18 if you 
were importing submodules directly, e.g. `lib/query/builder`.
@elhigu
Copy link
Member

elhigu commented Jul 7, 2019

Please check also if documentation repo needs changes when it builds knex. Some stub requires might need fixing.

@ricardograca
Copy link
Member Author

Will do.

@ricardograca
Copy link
Member Author

I checked and I can't find any reference to /src in the docs. I tried building it and it seemed to generate the docs correctly despite this error:

Building docs for production
[docs-server]  /home/ricardo/src/knex-documentation/node_modules/babel-core/lib/transformation/file/index.js:558
[docs-server]        throw err;
[docs-server]        ^
[docs-server]  
[docs-server]  SyntaxError: /home/ricardo/src/knex/lib/migrate/Migrator.js: Unexpected token (43:10)
[docs-server]    41 |       if (!knex.isTransaction) {
[docs-server]    42 |         this.knex = knex.withUserParams({
[docs-server]  > 43 |           ...knex.userParams,
[docs-server]       |           ^
[docs-server]    44 |         });
[docs-server]    45 |       } else {
[docs-server]    46 |         this.knex = knex;
[docs-server]      at Parser.pp$5.raise (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:4454:13)
[docs-server]      at Parser.pp.unexpected (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:1761:8)
[docs-server]      at Parser.pp$3.parseIdentifier (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:4332:10)
[docs-server]      at Parser.pp$3.parsePropertyName (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:4156:96)
[docs-server]      at Parser.pp$3.parseObj (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:4045:12)
[docs-server]      at Parser.pp$3.parseExprAtom (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3719:19)
[docs-server]      at Parser.pp$3.parseExprSubscripts (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3494:19)
[docs-server]      at Parser.pp$3.parseMaybeUnary (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3474:19)
[docs-server]      at Parser.pp$3.parseExprOps (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3404:19)
[docs-server]      at Parser.pp$3.parseMaybeConditional (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3381:19)
[docs-server]      at Parser.pp$3.parseMaybeAssign (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3344:19)
[docs-server]      at Parser.pp$3.parseExprListItem (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:4312:16)
[docs-server]      at Parser.pp$3.parseCallExpressionArguments (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3573:20)
[docs-server]      at Parser.pp$3.parseSubscripts (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3533:31)
[docs-server]      at Parser.pp$3.parseExprSubscripts (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3504:15)
[docs-server]      at Parser.pp$3.parseMaybeUnary (/home/ricardo/src/knex-documentation/node_modules/babylon/lib/index.js:3474:19)

I'll fix the conflicts now.

@kibertoad
Copy link
Collaborator

kibertoad commented Jul 10, 2019

@ricardograca Thank you! I will be releasing 0.18.4 shortly, and after that will merge the PR to 0.19.0 when PR is ready.

@ricardograca
Copy link
Member Author

Thanks! I just fixed the conflicts. You can ping me if there are more in the mean time.

@kibertoad
Copy link
Collaborator

Nope, seems to be just fine. I'll wait for CI to complete and then merge it. Thank you again!

@kibertoad kibertoad merged commit 9aa7085 into knex:master Jul 10, 2019
@ricardograca ricardograca deleted the general/move_src_to_lib branch July 11, 2019 09:07
@ricardograca
Copy link
Member Author

Thanks! I didn't realize that was just an example in the UPGRADING.md file 😅

@ricardograca
Copy link
Member Author

There seems to be one error in the test suite when running in Node 8. This wasn't happening before. Is it a known issue?

@kibertoad
Copy link
Collaborator

@ricardograca Difficult to check logs on mobile, but wasn't it #3225?

@elhigu
Copy link
Member

elhigu commented Jul 11, 2019

All seem to pass in master at least.

@ricardograca
Copy link
Member Author

@kibertoad I restarted the job and then deleted the branch (oops) so I can't see the original log or re-run the test, but it was one test complaining that an array of length 1 was expected but got 0.

In any case, if master passes it's all that matters.

felixmosh pushed a commit to felixmosh/knex that referenced this pull request Jul 13, 2019
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