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

Can no longer see NpmTask.result #237

Closed
JamesXNelson opened this issue May 4, 2022 · 4 comments
Closed

Can no longer see NpmTask.result #237

JamesXNelson opened this issue May 4, 2022 · 4 comments
Milestone

Comments

@JamesXNelson
Copy link

It used to be possible to look at the .result of an invoked NpmTask, but upon updating to latest version, I see nothing is recording the returned exec result

    @TaskAction
    fun exec() {
        val command = npmCommand.get().plus(args.get())
        val nodeExecConfiguration =
                NodeExecConfiguration(command, environment.get(), workingDir.asFile.orNull, ignoreExitValue.get(),
                        execOverrides.orNull)
        val npmExecRunner = objects.newInstance(NpmExecRunner::class.java)
        npmExecRunner.executeNpmCommand(projectHelper, nodeExtension, nodeExecConfiguration, variantComputer)
    }

please store the result object from executeNpmCommand somewhere visible to buildscripts (on the task), so smart builds that want to inspect errors and make helpful suggestions can do so.

@deepy
Copy link
Member

deepy commented May 5, 2022

The history of why this was removed is in #144 (comment)

Would the proposed alternative in #144 work for your use-case?
The branch is still around and I've planned to finish and merge it Soon™

@JamesXNelson
Copy link
Author

As long as I can get "something that has the state about the task" after it fails, I think I can survive...

It's not 100% clear what the proposed workaround is, an API for sending hooks to later receive the result object?

I think I can survive w/ that... but, I feel like the original change "hey, lets remove the result from the task b/c stateful things are bad" is one of those "I'm making changes because I believe in a design pattern" rather than "I'm making a change that is going to improve how people can use this code".

Tasks already are stateful. The are a big bag of state, they literally have a .state field describing their current state.

@JamesXNelson
Copy link
Author

Sorry for the complaining before... is there anything I can do to help make this happen?

@deepy
Copy link
Member

deepy commented Jun 9, 2022

No worries, the branch in #144 has suffered from the passage of time and doesn't merge cleanly any longer and I currently have limited spare time, I'll happily review and accept PRs. (for either solution)

The only hard requirements currently is that it works on 5.6.4 and that it doesn't break the configuration-cache.

Next version is going to be a minor but after that I might have to drop 5.6.4 from the main development.

deepy added a commit that referenced this issue Oct 17, 2022
@deepy deepy added this to the 3.5 milestone Oct 17, 2022
@deepy deepy closed this as completed Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants