-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Allow for NpmBuild install output to be logged #33
Comments
For at least npm commands run from bash it's also possible to use "command": "bash script.sh >> compile.log" to redirect stdout to a file. Though I'm not to sure how this would behave on windows and mac. I would strongly suggest though to write to the cargo log instead. |
Hi, yes, I changed this at the latest release, if it is really blocking you - just switch to previous version. Why I changed this: seems like (but I am not 100% sure, I would probably double check) this prevents change detection instructions to work. Because latest release adds exactly this feature, and this is really nice feature - allows really make builds and IDE faster, I decided to discard log output. But I think you suggestion about logging to file is nice, I will experiment on this. Just for curiosity, do you find NpmBuild functionality important, or maybe we should just drop this? I received recently request to drop it completely, want to know some feedback from users. |
Thank you for the fast reply. My build is luckily not blocked because I can redirect all output (from stdout) to the file with above mentioned redirect. I personally find NpmBuild functionality pretty important for new users since using npm, or webpack for these matters, is very popular and most likely the reason someone would want to embed their static assets in their binary. Though I can also understand why it may seem unnecessary for some users that don't rely on npm or another package manager. I would guess making it a default feature instead would be a possible option to simply give developers to choice. Though said that I really appreciate you caring about IDE performance. I didn't know that the output could affect IDE's though it certainly makes sense to drop the output then. Maybe some note on the readme that output from e.g. webpack will not be shown would be nice though (If it isn't already there and I didn't see the note). |
IDE performance is actually a thing, which surprised me also. But it really happens. It is a side effect, but I was struggling with this for a while, and last version was just a very first step in this direction. It does not work automatically at the moment to not break backward compatibility. I plan to release version 3.1 with some breaking changes in default options. We will find a way, to improve the user experience, for me it is most important feedback and I think it is really valuable. |
Happy to hear your plans for future releases. I personally support breaking changes, if they solve a problem, and I'm eager to test the new release when it's published. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I think we fix this soon. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Will close this for now since a mitigation has been found (piping output of commands to files) |
At the moment all output (to stdout) is discarded and unreachable for a developer. The appropiate function, from my POV, would be
NpmBuild::install
.I would love to either specify some file to write the log or to have it output to stdout.
Outputting to stdout or some kid of logger would allow the output to be shown in the cargo debug logs. By defualt there's no need to do any further catching as cargo will prevent any logs to be shown by build scripts.See this issue for more information on this topic.
The text was updated successfully, but these errors were encountered: