Skip to content

Instantly share code, notes, and snippets.

@amihaiemil

amihaiemil/mgramin_sql-boot_review.md

Last active October 24, 2017 06:47
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save amihaiemil/239169ef959fc064307e06aef6d30fd4 to your computer and use it in GitHub Desktop.
Save amihaiemil/239169ef959fc064307e06aef6d30fd4 to your computer and use it in GitHub Desktop.
Download ZIP
2017 Award Review for mgramin/sql-boot
Raw
mgramin_sql-boot_review.md

This file contains the review of the codebase as it was on 01.10.2017:

Project: https://github.com/mgramin/sql-boot

Language: Java

Main Contributor: https://www.github.com/mgramin

CLOC (cloc . on the root directory):

119 text files.
119 unique files.                                          
52 files ignored.
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Java                            93            757           1891           2499
Maven                            1             37             23            266
SQL                              4             29              0            109
YAML                             3              7              2             49
XML                              3              6             73             47
HTML                             1              0             23              1
-------------------------------------------------------------------------------
SUM:                           105            836           2012           2971
-------------------------------------------------------------------------------

Initial commit: 10 Apr 2016

Quick overview (+/- mean ups and downs)

  • (+) Ticketing + ticket referencing
  • (+) milestone set on each ticket
  • (+) Code coverage calculated and decent (> 70 %)
  • (+) Knowledge of the used framework; doesn't confuse it with JavaEE
  • (+) Every class has an interface
  • (+) No getters/setters POJOs
  • (+) Class naming
  • (+) Quick user guide and live demo options. Also, a lot of badges dislaying various states.
  • (-) project has less than 10k LOC
  • (-) no PRs, writing on master
  • (-) There are 3 releases so far, but no automation for them/
  • (-) Even though there is a Maven profile for qulice, it is not run during CI builds
  • (-) Packaging of the classes.
  • (-) No technical documentation and no proper javadocs

Detailed view

"7 Deadly Sins" Status:

1) Traceable changes/Github tickets:

There are tickets in the github repo, most of them opened by the owner for tracking purposes. However, there are no Pull Requests, the author simply commits on master and adds the commit message fixes #123 -- this references the issue and closes it automatically, it's a Github feature.

  • (+) ticketing
  • (-) no PRs, writing on master

2) Release process

The project has 3 releases (1 after 94 commits, 1 after 76 commits and 1 after 34 commits) -- they seem regular, but I didnt notice any automatisation of the process. It seems that he simply uses the Github release button.

  • (+) Regular releases
  • (-) No automation

3) Static analysis The project has a Maven profile for qulice (see here), but I do not see it being used neither in .travis.yml, nor in appveyor.yml

  • (+) Build profile for qulice
  • (-) Profile not run during CI builds

4) Test coverage

The project calculates its code coverage using Jacoco maven plugion and sends the report to Coveralls, via the coveralls Maven plugin. The coverage is only calculated in one of the CIs (which makes sense), in Travis. Also, there is a coverage badge on the main README.md file

  • (+) Code coverage calculated

5) Non Stop development

The project seems to be well structured, all the tickets have the milestone set. Also, there are 3 releases so far, seemed to be done at a more or less equal amount of commits. Also, the tickets are referenced in the commit messages.

  • (+) milestone set on each ticket
  • (+) regular releases
  • (+) ticket referencing

6) Patterns used

a) Framework: The main framework is Spring Boot, which is basically an alternative to JavaEE. To this regard, the author understands what Spring Boot is and uses it right, since the app is properly packaged with Maven Assembly Plugin, and run with Docker. There is a Dockerfile to run the application. This is very good, since I wouldn't have been surprised to see a Spring webapp ran on a JavaEE platform like Glassfish.

b) Code: As far as I can tell, every class implements an interface, which is very good. I've also noticed Fake implementations for the respective interfaces. There are no POJO classes either, didn't notice getters and setters. However, there are a few JsonProperty annotations, in order for Jackson (JSON library) to be able to parse the objects into JSON. Lombok is used for overriding toString. Naming of the classes is also ok -- just a few Controller classes, which are used by Spring to expose the REST api.

What I don't like, however, is the package structuring: each interfaces seems to have its package and then a child impl package, which holds the implementation. This seems too granular, and it's a sign to me that the developer wanted to just clearly structure the classes, without thinking of access modifiers and how these classes will access each other's methods, for instance.

E.g.: Interface FileSystem is in package: com.github.mgramin.sqlboot.tools.files.file_system, while its implementations are in package com.github.mgramin.sqlboot.tools.files.file_system.impl -- this is mostly the pattern for all the interfaces.

  • (+) Knowledge of the used framework; doesn't confuse it with JavaEE

  • (+) Every class has an interface

  • (+) No getters/setters POJOs

  • (+) Class naming

  • (-) Packaging of the classes.

7) Documentation

The project has a short, but fair documentation for the users, on the README.md (including options to try the project directly on Heroku). It also has a demo db and you can use Docker to spin up a demo.

However, I didn't find any technical documentation and the javadocs of the classes are rather poorn (some of them are even empty, only the @author tag is present). For me, as a developer, it would be very hard to start working on this, I have no clue of the overall architecture, how all those classes work together for instance. The only thing I understood was the entry point, which are those controller classes and Application class. For instance, I have no idea what is it with the folder config -- it only contains a tree of subfolders and some README.md files pointing to Wikipedia technical articles about topics such as Stored Procedures.

I would love to see an archirecture.md file, containing a quick overview of the codebase, maintypes, terminology etc.

  • (+) Quick user guide and live demo options
  • (-) No technical documentation and no proper javadocs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment