delete Matric Vector multiply rules that are now in ChainRules #828
Conversation
|
@oxinabox let's drop the version change and merge |
|
I am on leave. |
| @@ -1,6 +1,6 @@ | |||
| name = "Zygote" | |||
| uuid = "e88e6eb3-aa80-5325-afca-941959d7151f" | |||
| version = "0.5.14" | |||
| version = "0.5.15" | |||
There was a problem hiding this comment.
really we should avoid this, open PRs will continuously have conflicts with master, really annoying
There was a problem hiding this comment.
🤷 Its just not that big a deal.
it takes 30 seconds to rebase.
The person it annoys is the PR author, and that is me.
And I am not annoyed.
If other people don't want to rebase to deal with it then they don't have to bump the version number in the PR.
I am not here telling anyone else to.
On a small PR like this one the expectation is that it would be merged quickly (and this would have been except I was on leave and this PR was made before registry had the new version and so CI needed to be retriggered.).
and in a big PR the time to rebase because the version number has changed is nothing compared to the time taken to create the PR.
Further more, rebasing pulls in the changes from master.
And it is possible to end-up with a senario that has two PRs can both pass tests on their own, but not pass tests if merged one after the other.
Rebasing prevents that.
Some CI (definately Bors, idk about our github actions) also prevent that by testing on the result of the merge, rather than testing the actual code in the branch. Still for local tests it is nice the have rebased.
There was a problem hiding this comment.
I agree, just making sure we don't force this habit on contributors
|
Any performance considerations here? Otherwise lgtm |
Shouldn't be its for all intents and purposes the same code |
|
Gitlab failure is unrelated, some kind of trouble installing the package, i think i network blip at a guess |
@simeonschaub want to review?
this needs @gxyd 's PR to ChainRules to be merged first
JuliaDiff/ChainRules.jl#305