We built PullRequest to help developers. After waiting several days for feedback on a pull request while a colleague was on vacation, I knew there had to be a way to improve this process. Our mission is to improve code quality and save time for dev teams. We combine static and linting tools with real on-demand reviewers to help augment your current code review process. Dev managers like extra coverage, but our real intent is to free up developers to make better software more efficiently
We’re onboarding experts across a lot of different languages for this reason. Sometimes teams might only have one person working within a given framework/language – it can be difficult to get objective feedback before shipping to production if you’re working on an island.
All reviewers sign NDAs to protect your IP. We start with surface level reviews – complying with framework or language standards, algorithmic work, performance or other questions. Since our reviewers continue working on the same projects, they will also gain context for deeper reviews.
Looking forward to hearing your thoughts and feedback!
1. Style/consistency, re-use of existing code, utils, etc.
2. Architecture/design, how does this fit into the rest of the codebase, scaling concerns, how will the deploy work, will this have race conditions, etc.
3. Knowledge sharing with other members of the team.
Currently, it looks like this would satisfy half each of 1 and 2, but will miss the (possibly large) amount of context that people working on the project have. To be honest, I don't know how you solve that. How does a reviewer who lacks knowledge about the codebase spot a common pattern and know that another dev abstracted that out into a util a few weeks ago, for example.
I also wonder what could be done to address (3). I've seen the team I work on go from a place where everyone could review everything to a place where I can't review all the code that goes live, and particularly after time off, I can't really catch up. I'd love to see some sort of automated changelog of useful notes on what has changed. I'm not sure if this is possible, but summarising merged PRs, highlighting config changes, showing new utilities that have been added, etc, would be quite valuable.
> 1. Style/consistency, re-use of existing code, utils, etc.
> 2. Architecture/design, how does this fit into the rest of the codebase, scaling concerns, how will the deploy work, will this have race conditions, etc.
> 3. Knowledge sharing with other members of the team.
> Currently, it looks like this would satisfy half each of 1 and 2
In my opinion, Even a service that address the 1) and 2) add lots of value for so many IT projects (out side of cutting edge Startup companies )
Especially when they currently have no existing code review.
Could be fascinating to highlight config/code/approach changes on each pull request. Could actually help velocity across entire teams.
My workflow (which I believe is pretty standard) is:
* Write code
* Verify that tests pass locally (including stylistic tests, linting)
* Submit pull request
* Pull request triggers build and tests on Travis
* If all tests pass on Travis, code is stylistically and functionally correct
* Merge pull request
How can human reviewers improve this workflow?
Passing tests don't prove that
- tests are covering all functional criteria required
- the code doesn't add 'technical debt', i.e. structural problems that will have to be refactored/worked around in the future.
- check for whether external or downstream services would be affected (in the absence of complete e2e testing)
- check for coding conventions and standards that are not enforceable by linters, such as naming conventions, code structuring, positive/negative testing, effective usage of helper methods
- check for typos
I can write code that reinvents the wheel, instead of using the solid library I've never heard of.
I can do things in non-idiomatic ways that testing and linting will never catch.
A PR is about showing the rest of the team the changes so more than one person knows how stuff work and what's going on in the code base. And for the rest of the team to give feedback on stuff like how the feature was architected, not to nitpick on indenting.
But that's not really applicable here, right?
We offer a free review to companies interested in trying us out. Shoot me an email (lyal@ our domain name).
Having deep understanding of the code in question is essential for a good code review. Not just the code under review, but the wider scope of the project. This helps spot architectural problems, inconsistencies, unearth hidden assumptions or assumption breakages, and the like.
Reviewing the code as a drive-by loses all of those benefits and boils down to focusing on the code at hand, coding style, nitpicks, and implicitly assuming the code fits well with the rest (enforcing consistent coding style and pointing out code smells is certainly useful, these however can be automated to some extent by linters and services like CodeClimate).
I have been a reviewer in hundreds of pull requests, and reviews I've done where I have been intimately familiar with the existing code base were consistently much better than the reviews I did as an outsider to the project - even when, knowing this, I spent a lot more effort on the reviews as an outsider.
The founders seem to recognize this (it's mentioned in the TC article) and mention pairing up reviewers with the same companies, but this IMHO will not be enough, unless these reviewers are basically on retainer and work regularly, and often, with the same company.
I'd love to be proven wrong, so good luck PullRequest team!
Things like not know about certain shortcut functions in the standard library, improving design pattern usage, docstrings, and otherwise improving modularity, decomposition, cyclomatic complexity, consistency, etc. Code Climate goes far but doesn't do all of these things as good as an experienced engineer.
Although for teams that have a great reviewer like you mention with context, we think that PullRequest offers an extra set of eyes instead of a full replacement. We hope that we can hopefully save you time and add value by catching all of the mistakes possible up front and leave the architecture up to you.
We'll definitely have free tiers for our static and instrumentation product though.
Incentives and gamification are definitely there as well. We want to bonus people for doing thoughtful code review.
I also think that this seems absurdly cheap, and I can't imagine it scaling with quality reviewers. Would love to be wrong on this one.
Does that mean I could expect to pay < $49/mo for a learning use case? These projects don't move to quickly, so I envision using static analysis extensively, then getting a more complete review 1-2 times a month.
Might be worth clarifying the pricing description.
There are things that a human can suggest that computers can't. Such as a refactoring suggestion.
Here are a few ideas:
- Consider adopting a standard like EditorConfig (http://editorconfig.org/) for reviewers to have compliant indentation out of the box
- For Enterprise packages: perhaps there can also be an opportunity to sub-contract out features and write tests?
- Consider experimenting internal CI tools (like as done in open source projects) to scan for obvious/low-hanging fruit automatically
- Scanning for / suggesting package updates
- Provide QA / audit for a large open source project for exposure
- Security auditing
Here are things that are good to hear:
- Static / Linting: things like vulture, flake8, etc. seem like a nice thing to stick to. It's good that these linters have configuration files to it
Completely agree re: editorconfig. Very necessary to prevent bikeshedding. We're actually building a dedicated review IDE.
Part of our roadmap is to offer open source projects code review -- not just for exposure, but to work with reviewer standards on.
We're definitely interested in security review.
Edit to expand:
We also believe that reviewers attached to projects will gain context quite rapidly. We had a reviewer catch an edge case bug for one of our teams that had gone unnoticed by internal review. Economic side of this would have been large... but was only possible through the context they gained in previous reviews.
It would be nice to see a demo video before giving full access to my private repos.
> Pricing > Standard starting at $49 per month*
> * Billing is dependent on amount of meaningful change per month. $9 per user per month for static analysis.
This metric is pretty unclear. Does this mean hourly billing based on reviewer time? Are there tiers or an upper bound? Is there a different tier for open source? Is the pricing different for surface vs deep reviews?
As one of those weird people that thinks doing code reviews and managing code quality is really fun, if I wanted to become a reviewer, what's the vetting process like?
Can you elaborate on, besides involving humans, how the underlying service is different than Code Climate, Codacy, etc?
P.S. Found a small bug on your dev signup form which I reported on Twitter. It would be awesome to be able to help review PullRequest using PullRequest ;).
All the issues someone with no familiarity of the code base or the problem could typically uncover are things that are prone to be automated away by software in the long run (or are already in the process of being automated).
How does your company back this up? What happens if one of your Developers violates this? Will you pay for the legal fees?
Another good idea is for new programmers in a production environment just having an eye over their shoulders making sure they aren't making rookie language mistakes (simpler ways of doing etc.). Letting official company engineers focus on architectural/roadmap related issues.
This, to me, would be the fastest way to learn as well.
