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

Don't parse npm scripts for dynamic config #1990

Merged
merged 2 commits into from
Sep 24, 2018
Merged

Don't parse npm scripts for dynamic config #1990

merged 2 commits into from
Sep 24, 2018

Conversation

maturanomx
Copy link
Contributor

Fix the fix merged with #1575

The original patch was done over a block of code moved on #1667 resulting in re-introducing code that now is not invoked and continue with the bug reported on #916.

This patch take the original changes and put them in the current correct place.

Proof:
capture


My PR is a:

🐛 Bug fix

Main update on the:

Plugin

@maturanomx
Copy link
Contributor Author

maturanomx commented Sep 20, 2018

Travis fails. This is from the log:

📦  Linking strapi-lint
npm ERR! code E503
npm ERR! 503 No backends available: @xtuc/long@4.2.1
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2018-09-19T23_36_07_284Z-debug.log
npm ERR! code E503
npm ERR! 503 No backends available: @xtuc/long@4.2.1

In my local I also have problem with exactly the same dependency, just I was getting a 404 error. Temporary problems with the registry? coincidence is the same library? is a new dependency? 🤔

@lauriejim
Copy link
Contributor

I have restart tests, looks working well.

@lauriejim lauriejim self-assigned this Sep 21, 2018
@lauriejim lauriejim requested review from lauriejim and Aurelsicoko and removed request for Aurelsicoko September 21, 2018 15:52
@lauriejim lauriejim added pr: 🐛 Bug fix source: core:content-manager Source is core/content-manager package labels Sep 21, 2018
Fix the fix merged with #1575

The original patch was done over a block of code moved on #1667
resulting in re-introducing code that now is not invoqued and continue
with the bug reported on #916.

This patch take the original changes and put them in the current correct
place.
@maturanomx
Copy link
Contributor Author

maturanomx commented Sep 21, 2018

I have restart tests, looks working well.

Oh!, you mean the Travis tests :)

Just for add information, this is from master reproducing #916 :
capture

@lauriejim lauriejim added this to the 3.0.0-alpha.14.2 milestone Sep 24, 2018
Copy link
Contributor

@lauriejim lauriejim left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you

@lauriejim
Copy link
Contributor

Have to check for the issue you screenshot.

@lauriejim lauriejim merged commit 297e52e into strapi:master Sep 24, 2018
@maturanomx maturanomx deleted the fix/916 branch September 24, 2018 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
source: core:content-manager Source is core/content-manager package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants