I don't understand why people are against this so much. Black does a sanity check and compares the AST before and after to make sure there aren't any meaningful changes (unless you are running it with --fast). So there is almost no risk that it will break your code.
There is nothing more frustrating than coming back from a coffee break only to find out that you have to rerun your CI check because of a trivial formatting issue.
If you use feature branches, it is quite annoying that can't push two consecutive commits because CI changed something in your branch and now you have to resolve conflicts.
We have it set up to only run after you've opened a PR. You shouldn't run into the issue often that way because if you're opening a PR then all your big immediate changes are already committed so you won't be pushing another commit until someone reviews which will be after black has already finished.
Sometimes I still sneak in minor changes after I opened a PR. Sometimes I open PR early because CI has integration/e2e tests that are hard to run locally. Sometimes I want feedback on certain parts early on and PR is the easiest way to show something.
There can be workflows in which pushing through CI works fine, but as a general advice it's not great because there are many edge cases.
While it hasn't been updated in a while, fundamentally https://github.com/pombredanne/django-zerodowntime would be a base for building similar check logic in a DB agnostic way on Django. It has some of the most important checks in place, but could obviously do with adding more sophisticated checks.
It might be just me, but I feel like remembering the foreign key name is more difficult than remembering the columns that you need in the ON clause. Especially since you can usually find the column names by just seeing the data in the table (select * from x) wheres seeing the foreign key names is much harder (show create table x?).
Also, if you use an ORM it will usually generate foreign key names that are almost impossible to remember.
In a universe where foreign key index names are important we would specify better names.
I think stuff like “documents_by_user” as foreign key names and explicit index usage would improve peoples awareness of how indices get used and would generally be a positive
I think this is an operator problem. You're using the wrong tool for the job.
TablePlus, SequelAce, the official MySQL client all support cntrl-space autocompletion. I wish we used Postgres, but I imagine the landscape is the same. The big box databases like Oracle, DB2 undoubtedly having this tooling as well.
That being said, here is our fk naming convention: `fk-asset_types->asset_categories` which pretty states what's going on and is easy to remember.
SQL is not only written in an SQL client. SQL is also written (and read from) embedded/mixed in an other programming language were tooling is not always available.
Having to know the names of foreign keys (in addition to the column names of the 2 tables) is adding more cognitive load. I don't think that is an improvement.
At least when it comes to both JetBrains and VSCode, they can handle one language embedded in another. I'm kind of surprised there are environments that don't handle that these days.
It would indeed be difficult to remember, but the proposal also suggest changing the default naming convention for foreign keys, to give them the same name as the referenced table.
If using an ORM, I would guess this proposal isn't useful, since then you wouldn't hand-write queries anyway, right? Except when you want to override the queries generated by the ORM? (I'm not an ORM user myself.)
Speaking as someone who has used ORMs in the past and contributes to a LINQ Micro ORM...
It might make tooling 'easier', but since backwards compatibility has to be considered the actual value add is questionable IMO.
Most ORMs/MicroORMs will have tooling that sniffs out the DB Schema including foreign keys, and if you are using those bits (i.e. 'not hand written') most will do the right thing today. I suppose you could include some extra syntax for whatever DSL you're providing users....
IDK. Speaking as someone who is very comfortable in SQL, This feels more like syntactic sugar than anything else.
I have worked on a Google project as a contractor and I can confirm that over-engineering was our biggest problem. I was also partially to blame, but at the time I did not know better.
To fix this problem you can set up you CI server to merge into the base branch automatically before running the code (in fact you should probably do this by default for other reasons).
This way your devs won't have to merge, they can just rerun their tests, which should be the same workflow as if your CI config is separate from your codebase.
I've been using GitHub actions for my personal projects for the past few months. The documentation was not the best and the UX was not polished at all, so the experience was painful sometimes. A lot of my problems will be solved with the features announced today, except I'm not sure if they're also adding a way to cache e.g. dependencies between jobs.
While writing your own actions was painful (at least for me), reusing actions that other people wrote worked like magic. I think the reusability aspect is going to be huge when Actions get more and more popular.
The article makes some good points. Here are my thoughts:
> 1. Preliminary Checks
+1 for this, but I would also add that you need to add as much formatting/linting into this step as possible. You shouldn't be pointing out formatting issues in your code review.
> 2. Understanding
This is super important! Pull requests that do not explain their purpose in the description should not be reviewed.
The PR description is also really important when git bisect leads you to the PR when debugging a new bug.
> 3. Usability Test
A lot of debate regarding this, but I believe this is extremely important if not the most important part of your code review. For details read this section[0]
> 4. Code Review
A few good tips in the article, I'd just add that you should be polite and try not to waste people's time.
> you need to add as much formatting/linting into [step 1] as possible
Yes, project formatting is a whole other topic but this assumes the project already has the team's linting/formatting rules baked into the build. As a reviewer, we are pointing out failing builds here, whatever they may be. If there are deeper issues/disagreements with the build tasks, that would be a separate issue.
Actually, pair programming is a way of doing code reviews. Some teams consider pair programmed code as already reviewed. I don't have enough experience with pair programming to confirm this, but it is an interesting idea.
I felt the same way as you do, I used the git CLI directly and could never use a git GUI.
But then I figured out that Atom's git integration is actually quite neat and I started using it mostly to do `git add -i` as it makes staging files or parts of files really easy.
I still don't trust it with pushing/pulling, switching branches etc. so I still drop to the command line for those, but the atom git integration is incredibly useful for seeing your changes and committing a particular part of them.
I agree. I use the command line for everything but staging, and I often only stage parts of files while discarding a few leftover pieces I forgot about. I use sourcetree, but the concept is the same. GUI git is really nice for staging parts of changes. And then I usually commit via the GUI too, since I'm in there anyways.
There is nothing more frustrating than coming back from a coffee break only to find out that you have to rerun your CI check because of a trivial formatting issue.