Contributing standards?

[wlach]

Just wanted to touch base about this community fork of redash (thanks everyone here for starting it!). My employer (Voltus) uses redash internally so I have some interest and a bit of time to help. I've already submitted a few PRs:

https://github.com/RedashCommunity/redash/pulls/wlach

I'd be interested in switching us to using this fork, but I'm a little concerned about some of the PRs/commits that have been going into it. It looks like there's often no review, and the commit messages make it difficult to see what changed (e.g. changesets with the line "Code optimization"). I am sensitive to the fact that not everyone has english as a first language, but on the other side of the spectrum a commit history of undocumented and/or questionable changes makes it hard for me to be sure that using a piece of software won't break something (or cause a security incident).

I'm just curious if this group would be able to agree on some contributing standards? There were some IMO pretty good ones that are already in the repo: https://github.com/RedashCommunity/redash/blob/master/CONTRIBUTING.md - - we might want to add one or two things to it, but I think it's a reasonable place to start. I think the most important thing is not allowing any changes to go in without review-- this might slow things down slightly, but I think it'll give us more confidence that the code quality is kept up.

In addition I might suggest only allowing squash merges (to keep the history clean) -- GitHub has a setting for this.

[justinclift]

@wlach Yeah, things are a bit messy in the repo at the moment. It seems like @gaecoli and likely yourself are reasonably familiar with the code base, so have a mature perspective on how the bits fit together.

On the flip side, @Avey777 and myself are sort of unsure about a lot of the pieces. @Avey777 is still getting the hang of some of the technical git stuff too (thus messy commit history), but that should come good in a few days (my rough estimate).

So... it's probably a bit early to start holding ourselves to the same standards as the main Redash repo. But, we'll probably be closer to that in a few weeks, after we get the hang of things more.

Hopefully that makes sense? 😄

[wlach]

Yeah, totally understand there's a learning curve here. I'd still recommend adding a review requirement and only allowing squash commits in the repository settings (https://github.com/redashcommunity/redash/settings):

At the very least I'd disable "merge commits" which are IMO a bit of a gotcha and often the source of a strange-looking commit history.

[gaecoli]

I think everyone have to know git rebase.

[justinclift]

Oh, I didn't realise it's possible to disable merge commits. That's awesome. Yeah, I'll disable that right away. 😄

The review thing is kind of borderline for the moment. It's useful for the Python and frontend JS/TS code, but gets in the way of @Avey777 and myself updating dependencies (which we're focused on atm).

Once we have the dependencies where they need to be, it's probably a good idea to turn the review requirement back on then. 😄

[justinclift]

I've just applied these changes:

This should mean our repository history is clearer from here onwards. 🕺

[Avey777]

Just wanted to touch base about this community fork of redash (thanks everyone here for starting it!). My employer (Voltus) uses redash internally so I have some interest and a bit of time to help. I've already submitted a few PRs:

https://github.com/RedashCommunity/redash/pulls/wlach

I'd be interested in switching us to using this fork, but I'm a little concerned about some of the PRs/commits that have been going into it. It looks like there's often no review, and the commit messages make it difficult to see what changed (e.g. changesets with the line "Code optimization"). I am sensitive to the fact that not everyone has english as a first language, but on the other side of the spectrum a commit history of undocumented and/or questionable changes makes it hard for me to be sure that using a piece of software won't break something (or cause a security incident).

I'm just curious if this group would be able to agree on some contributing standards? There were some IMO pretty good ones that are already in the repo: https://github.com/RedashCommunity/redash/blob/master/CONTRIBUTING.md - - we might want to add one or two things to it, but I think it's a reasonable place to start. I think the most important thing is not allowing any changes to go in without review-- this might slow things down slightly, but I think it'll give us more confidence that the code quality is kept up.

In addition I might suggest only allowing squash merges (to keep the history clean) -- GitHub has a setting for this.

You are right. I will learn and use the new way of submitting in the future.