sciml / colprac Goto Github PK

Guidance on contributing PRs says:

You should not squash down commits while review is still on-going.

• Squashing commits prevents the reviewer being able to see what commits are added since the last review.

It is not quite clear to me, whether this includes amending commits and then force-pushing. This is probably something that should be discouraged for non-trivial changes, but IMHO amending makes sense, if you realize right away that you made a dumb mistake and want to push a quick fix. Is that something that should be added? Do people have different opinions on this?

In https://github.com/SciML/ColPrac#dropping-support-for-earlier-versions-of-julia it says it’s a non-breaking feature. But pre-1.0 is that a minor bump or patch? The guidance doesn’t seem clear which to choose. I would advocate for patch.

If a PR is made and approved, and merged into the main branch.
Then if the backport is fairly trivial,
then the PR to backport it to another branch does not need separate approval.
Since the content has already been reviewed and approved.

This is to track rolling this out across organizations,
a lot of PRs need to be made to add this to README.mds

for convinence here is the markdown for the badge

[![ColPrac: Contributor's Guide on Collaborative Practices for Community Packages](https://img.shields.io/badge/ColPrac-Contributor's%20Guide-blueviolet)](https://github.com/SciML/ColPrac)

• JuliaCollections
• SciML
• JuliaDiff
• FluxML

Feel free to add others if we also need to track them

I think it might be useful to define what a non-user visible change is.
Possibly with reference to making releases.
In a sense perhaps a non-user visible change is a 3rd category, like bug-fix or feature.

When I say non-user-visible change I mean one of:

• Refactoring to change internal structure (but not the public API)
• Updating a comment
• renaming a variable
• Fixing a mistake in the tests

Some things sound similar like:

• Performance improvements
• updating documentation.

But are not.

Was reminded of this in
JuliaDiff/ChainRules.jl#296 (comment)

There are some things that should be done before tagging a breaking release.
This is what i have so far:

• Are you sure this is breaking? Tagging breaking releases that are not actually breaking just adds work for everyone.
• Increment version number / remove -DEV suffix
• Delete any deprecations added since last nonbreaking release (don't forget to search for uses of depwarn that might be scattered in the code)
• Write release notes and/or upgrading guide if appropriate.

Right now the package does not allow kernel matrix calculations with CUDA arrays which have type CuArray
For example, the following code breaks

using KernelFunctions, CUDA
x = CUDA.randn(10, 2);
z = CUDA.randn(100, 2);
k = GaussianKernel();
K_xz = kernelmatrix(k, x, z, obsdim=1);

It seems to me that the code can work just as well with CuArray from CUDA.jl

Can this functionality be added. As far as I can tell, we just need to add new method with CuArray as input data type.

It may be worth clarifying that if a PR is made by an automated system, e.g. CompatHelper.
Then that system is the "author" of the PR.
And thus it needs review only from someone with merge rights.
Even if it is a automated system that that person triggered directly/manually.
As long as the system itself is well tested and good.

@ChrisRackauckas mentioned he was thinking it might be good to have a simple webpage for this.
Something that just renders the markdown from the README.md