Skip to content

Pipeline fixes#93

Open
LourensVeen wants to merge 5 commits into
NLeSC:masterfrom
LourensVeen:pipeline-fixes
Open

Pipeline fixes#93
LourensVeen wants to merge 5 commits into
NLeSC:masterfrom
LourensVeen:pipeline-fixes

Conversation

@LourensVeen

Copy link
Copy Markdown
Member

This one is a bit more scary than my previous contributions. Commit 511c1b4 mostly rewrites pipeline(). As far as I can tell the original is actually correct, but determining that was not so easy. The new version should be doing the exact same thing, but is more obviously correct. It passes the unit tests, but is otherwise untested. Please review carefully.

@LourensVeen

Copy link
Copy Markdown
Member Author

No more map(), got it :-).

@LourensVeen

Copy link
Copy Markdown
Member Author

Improved documentation while we're at it. The terminology still isn't completely clear to me, is there a difference between a task and a module?

@larsmans

larsmans commented Apr 4, 2016

Copy link
Copy Markdown
Collaborator

"module" should have been "task".

@LourensVeen

Copy link
Copy Markdown
Member Author

Do you want to change it, or leave the API as is? Or accept both?

@larsmans

larsmans commented Apr 4, 2016

Copy link
Copy Markdown
Collaborator

Let's change it but accept the old format as well. We can emit a warning if users use the old format.

@LourensVeen

Copy link
Copy Markdown
Member Author

Added a patch to now use "task", but also accept "module" and emit a warning in that case.

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

Successfully merging this pull request may close these issues.

2 participants