-
-
Notifications
You must be signed in to change notification settings - Fork 329
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
Do not deduplicate for auto-complete #443
Conversation
Nice thank you, I will integrate this one |
Hummm... The build fails when building the container. Does it even happened? If not I'll have a look at it tomorrow. |
@FaustXVI I will merge this pull request, failing build is not related to your patch |
If you rebase on top of master, it will work |
I did a big refactoring on the code base. I will merge this one tomorrow |
No big deal. I'll rebase this evening.
Xavier Detant
…On Wed, 17 Jul 2019, 17:28 Guillaume Vincent, ***@***.***> wrote:
I did a big refactoring on the code base.
It breaks your pull requests.
I will merge this one tomorrow
sorry
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#443?email_source=notifications&email_token=AAHYIH5WZV6HFK5EUJVNI23P743CPA5CNFSM4IECV64KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2EYX3A#issuecomment-512330732>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHYIH6SVW2YCQHIN5U4DMDP743CPANCNFSM4IECV64A>
.
|
eb6e111
to
d87c6fb
Compare
29e54e9
to
c24ce48
Compare
@@ -19,6 +19,8 @@ | |||
"@babel/plugin-transform-react-jsx": "^7.3.0", | |||
"@babel/preset-env": "^7.5.4", | |||
"@babel/register": "^7.4.4", | |||
"@vue/test-utils": "^1.0.0-beta.29", | |||
"babel-core": "7.0.0-bridge.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you removed it but it breaks vue-jest (vuejs/vue-jest#160).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put it back if you need it
@@ -19,6 +19,8 @@ | |||
"@babel/plugin-transform-react-jsx": "^7.3.0", | |||
"@babel/preset-env": "^7.5.4", | |||
"@babel/register": "^7.4.4", | |||
"@vue/test-utils": "^1.0.0-beta.29", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that you moved all dev dependencies to the top level, so I did the same. Am I right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you're right
"transform": { | ||
".*\\.(vue)$": "vue-jest", | ||
"^.+\\.js$": "../../node_modules/babel-jest" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I move all this to the top level ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be ok to keep jest config in this package.json no?
I don't know if it's a good practice or not, but some dependencies are duplicated. So yes you could move those dependencies at the root level |
Thank you @FaustXVI |
Closes #333
If this PR is ok, another one will follow in order to print the login next to the site name