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.

Sign up for GitHub

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

Jump to bottom

Make it possible to pass closure to GooseTask::new #120

Merged
merged 6 commits into from
Jul 29, 2020
Merged

Make it possible to pass closure to GooseTask::new #120

merged 6 commits into from
Jul 29, 2020

Conversation

LionsAd
Copy link
Collaborator

@LionsAd LionsAd commented Jul 28, 2020 •
edited
Loading

This works perfectly now - as proven by the CI runs.

The tests error I first ran into is described in #122 and led to a stack overflow error, but was unrelated to things.

Closes: #76

@LionsAd LionsAd force-pushed the issue-76--make-it-possible-to-pass-closure-to-goose-task-new branch from 3e54a4a to 49ac4c4 Compare July 28, 2020 16:37
@LionsAd LionsAd force-pushed the issue-76--make-it-possible-to-pass-closure-to-goose-task-new branch from 49ac4c4 to 373511e Compare July 28, 2020 19:51
@LionsAd
Copy link
Collaborator Author

LionsAd commented Jul 28, 2020

This is so weird -- test fail locally for me, but pass on CI ...

LionsAd
LionsAd commented Jul 28, 2020
View reviewed changes
Copy link
Collaborator Author

@LionsAd LionsAd left a comment

Choose a reason for hiding this comment

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

Some comments

LionsAd
LionsAd commented Jul 29, 2020
View reviewed changes
jeremyandrews
jeremyandrews requested changes Jul 29, 2020
View reviewed changes
Copy link
Member

@jeremyandrews jeremyandrews left a comment

Choose a reason for hiding this comment

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

Exciting to see this working! Please see my feedback above. We also need to performance test before/after to understand if there's a performance impact from this change.

@jeremyandrews
Copy link
Member

Performance comparisons between the master branch and your PR:

@LionsAd LionsAd force-pushed the issue-76--make-it-possible-to-pass-closure-to-goose-task-new branch from 1ca5b4c to aa20847 Compare July 29, 2020 10:56
@jeremyandrews jeremyandrews merged commit 3725dad into tag1consulting:master Jul 29, 2020
@jeremyandrews jeremyandrews mentioned this pull request Jul 29, 2020
Closed
@alsuren alsuren mentioned this pull request Mar 26, 2021
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeremyandrews jeremyandrews jeremyandrews approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

is it possible to pass paths programatically?
2 participants
@LionsAd @jeremyandrews