Merge code and SpotBugs issues

Posted in :

stlplace
Reading Time: 4 minutes
Sinking hole in our neighborhood main street? We try to find the root cause.

Back to coding. When working on the same file at the same time, the old joke is try to check in your code change before the other girl (guy) does. This way you don’t have to worry about the merge (meaning, put in your code change while keeping the other person’s code changes). In this day and age though, when working in a team, with the code review process, etc., we cannot count on we always be the 1st one to check in code changes. Thus, comes the “merge hell”. And I thought I was in that for a moment last few days, as I didn’t know a lot code has been changed for the repo (feature branch) I was reviewing and tried to merge. But it didn’t turn out as bad as I thought.

Some observations I have. Some are well known: one is don’t keep the feature branch for too long. I think the recommendation is a day or 2 (source 1, source 2). But we kept it much longer than that. To add to the complexity, I saw meaningful amount of refactoring being done in another branch (which I need to keep as well, at the same time it complicated my feature branch merges).

The approach I took eventually: initially I accepted everything in my feature branch, and I quickly realized that won’t work. So I took the opposite approach, I accepted everything in the dev branch (after the other feature branch is merged), and then I pick and choose my feature branch changes, and put in those changes. Ran local build and tests.

Btw, on testing front, I observed a locally hosted (localhost) Web Service (WS) calling another WS locally vs calling the WS hosted on #AWS #K8 has performance impact: roughly 15 sec vs 45 sec. I yet to test the scenario of caller WS on AWS.

My good old days

I cut my teeth on “code merge” about 20 years ago, when I was working for a software company. We didn’t have agile and all the jazz. But we were doing the modern day agile and did software dev in an increment way. Every 2 weeks different dev branches (teams) of the company merge our code changes to the central repo, and we call it integration. Since we have annual release of our software, we pretty much do about 20 phases (2 weeks x 20 = 40 weeks) for a release. Each team has a person that is responsible for the build and integrate the code to the central repo (not full time job, just once every 2 weeks, and the total time spent is about 8 hours the most for every 2 weeks). And yours truly was the integrator. And I had to resolve the merge conflict sometimes. And in some cases I need to get other teams involved to make sure my merge is good. I recall we used PerForce for source control, and KDiff3 at the time.

SpotBugs

Just as I thought I was close to be done. I encountered a few SpotBugs issues. Mainly there are 3 types, as shown below.

  1. Optional type returns null (this is a big no no). I took out the “Optional”.

2. The default getter and setter generated by Lombok has a potential security issue. The solution is to make a deep copy. Refer to this SO thread 1, thread 2 and thread 3. For me I found out I need to do a null check in the setter (we don’t want to set something to null, it throws null point exception too).

3. And last but not least, it didn’t like multiple instances of hard coded strings being created in the same file. This is a low hanging fruit – just create a string variable for that.

I think one common question may come up is how SpotBugs compares to Sonar Qube. My impression is SpotBugs can be run locally, and per the “fail earlier, fail faster” motto in the Agile development, this is excellent. As for Sonar (or Sonar Qube) my recollection is we usually configure it with a CI tool such as Jenkins, and we usually need to proactively look at the results ourselves: we likely can configure it so that at least it can email us the failures in a CI build. But either way it seems local build capability is either not there or hard to achieve.

G2 also has an executive summary. Quote below:

“Reviewers felt that SonarQube meets the needs of their business better than FindBugs. When comparing quality of ongoing product support, reviewers felt that FindBugs is the preferred option. For feature updates and roadmaps, our reviewers preferred the direction of FindBugs over SonarQube.”

Bonus Advice

I probably solved more coding problems by stepping away and taking a walk… or driving on the way to home after work. Once I couldn’t resist the temptation to try it out so I turned around 😂 #walking #driving #coding (warning: always pay attention to the surroundings while drive or walk, the physical safety of you yourself and those around the you are the most important).

(Update 01-Nov-2023) I found an issue with “merge commit”. I did not do the rebase or at least I did not use the git rebase command. What I did: I go to the “target branch” in my local, do a git pull (get the latest code), then go back to the “from branch”, do a git merge target_branch, now the “from branch” is updated with the “target branch” most recent code change, and I would no longer have “merge commit” in my Pull Request (from merge branch to target branch). Note the code change was pushed into the merge branch using the command “git merge feature_branch”. Note this is a 3 way kind of merge, from “my feature branch” to “from branch or merge branch”, and the code change eventually goes to “target branch”. We like to have only the relevant code changes (commits) from the “merge branch” to “target branch”. Read this doc if you like to learn more about advanced merge.

(Update 11-Dec-2023) Thought this one is funny. The gradle build on my local worked fine, but SpotBugs test failed on the CI (Continuous Integration) build. I don’t have access to the CI server. And I guessed it’s the unused variables I just added. So I added a bit loggings for the two new variables I just introduced. And it worked: now the CI build is good.

%d bloggers like this: