Skip to main content

Responsibilities of Solution Architect

– Responsible for Technology
– Converts Functional Requirements to Technical Architecture
– Carefull balances Patterns/Requirements/Elegance/Concepts
– Researches Key Technologies
– Has deep understanding of Design Patterns
– Motivates and guides development team
– Ensures that the Lead Developer is successful

Integration Spaghetti

I’ve been using the term Integration Spaghetti™ for the past 9 years or so to describe what happens as systems connectivity increases and increases to the point of … unmanageability, indeterminate impact, or just generally a big mess.  A standard line of mine is “moving from spaghetti code to spaghetti connections is not an improvement”.

(A standard “point to point connection mess” slide, by enterprise architect Jerry Foster from 2001.)

In the past few days I’ve been meeting with a series of IT managers at a large customer and have come up with a revised definition for Integration Spaghetti™ :

Integration Spaghetti™ is when the connectivity to/from an application is so complex that everyone is afraid of touching it.  An application with such spaghetti becomes nearly impossible to replace.  Estimates of change impact to the application are frequently wrong by orders of magnitude.  Interruption in the integration functioning are always a major disaster – both in terms of the time and people required to resolve it and in the business impact of it.

Even as the spaghetti bound application is nearly impossible to replace, as the current state continues it continues to grow worse as additional connections are made to these key applications and derivative copies of the data are taken from it, or clones created to avoid it (and thereby creating another synchronization and connection point).

Such spaghetti takes multiple forms but often involves ALL forms with multiple generations of technology connections, including excessive point to point connections, tightly coupled connection technologies, database triggers, business logic embedded in EAI process steps, many batches in and out from and to many destinations, ETL loads and extracts to/from other databases, multiple services providing nearly (but not exactly) identical data sets, and the involvement of many message queues.

Anything is done to avoid dealing with the giant plate of spaghetti.

Systems will integrate with systems that integrate with it, piggybacking existing connectivity and putting a burden on the subsidiary system, to avoid directly connecting into the spaghetti.  They’ll go to a secondary or tertiary data source to avoid going direct.  Everyone knows avoid the spaghetti if at all possible and will spend double to triple the integration effort to do so.

If the primary system is replaced, it’s not unusual that the new system won’t be integrated into all the old connections – this would require actually understanding each existing connecting, extracting it and redirecting/reconnecting it to the new system – rather the OLD SYSTEM will stay around to act as the connection point for all the existing spaghetti connections and the new system will become an integration, taking data feeds or a regular ETL load, off the old system!  Meaning the old system lives forever!

Does this problem every get resolved?  Yes.  When the other side of the connections gets replaced, the new systems on that side will be integrated with what replaced the core spaghetti bound system.  If the IT shop is lucky after a generation or so the spaghetti bound system can be shut down.

Unfortunately in major Enterprise IT shops finding some spaghetti integrations is not unusual.  IT management is loathe to acknowledge such a problem to the business and will continue work-arounds until it directly impacts business goals.  Otherwise it remains just another hidden IT enterprise IT expense.

source

 

Pre-merge Code Reviews

Rules, benefits, patterns, and anti-patterns for reviewing code before merging it.

Published on 26 October 2013 by @mathiasverraes

I’m a big proponent of pre-merge code reviews. From my experience consulting for teams in problematic projects, I can say that (along with daily standup meetings) pre-merge code reviews are one of the most effective and yet fairly easy changes a team can introduce to radically improve the condition of the project. And even if you consider your project to be healthy, there’s always room for improvement.

The Rules

  1. Never Push to Master
    Always push to a separate branch per logical unit (story, feature, bug, optimisation, refactor, improvement). Branches are easy to make and easy to merge when you use git (and you apply some of tips further down in this post).
  2. Never Merge Your Own Branch
    This helps to ensures that code is in fact reviewed. If you are caught merging into master, you will order pizza for the whole team.
  3. Review Work in Progress First
    When you are finished with a task, you notify the other team members that your work is ready for final review. Then you review existing branches. Before picking up a new task, you look at all open pull requests (including unfinished ones) and review the changes since the last time you checked.
  4. Merge responsibly
    Merging a pull request is the responsibility of the whole team. A pull request can not be merged when someone in the team does not understand the code or the reasoning, or does not agree with the solution.

(Note that these rules are starting points. Figure out what works in your team, adapt continuously.)

The Benefits

Having multiple sets of eyes review a pull request before it gets merged to master or an integration branch, is a great way to catch defects early. At this time, they are usually still cheap to fix.

There are however much more important benefits. Instead of individual developers, the team is responsible for the internal and external quality of the code. This is a great remedy against the blame culture that is still present in many organisations. Managers or team members can no longer point fingers to an individual for not delivering a feature within the expected quality range. You tend to become happier and more productive, knowing that the team has your back. You can afford to make a mistake; someone will find it quickly.

Another effect is something called ‘swarming’ in Kanban. Because you are encouraged to help out on other branches before starting your own, you start to help others finishing work in progress. Stories are finished faster, and there’s a better flow throughout the system. Especially when stories are difficult, or when stories block other stories, it’s liberating to have people come and help you to get it done.

And of course, there’s all the benefits from the clear sense of code co-ownership. It’s invaluable to have a team where everybody knows what the code does, why it’s designed that way, how everything fits together. It also reduces the Bus Factor: no single team member is a bottleneck. Best practices are shared, and the code is more consistent. Opportunities for reuse are spotted before lots of duplication happens.

In short, pre-merge code reviews grow the team’s maturity.

Making It Easier

Reading code is hard, much harder than writing it. Here are some ideas that I have found to make things easier.

  • Atomic Stories
    Break down stories in smaller, more atomic stories. “Manage products” is way to large. “Add products”, “Add an image to a product”, “Modify the description of a product”, “Remove a product”, “Set a new price”: these are atomic, and it will make branches much easier to review and merge.
  • Acceptance Criteria
    List the acceptance criteria in the pull request. (In GitHub, you can use checkboxes in Markdown, making it easy to tick them).
  • Atomic Commits
    Commit atomically. For example, don’t do code formatting and code changes in one commit, it makes the diffs very hard to read. Each commit should preferably leave the code base in a consistent state, but apart from that, there’s no limit to how small the commits can be. I often commit single lines.
  • Pull Requests as Conversations
    A pull request is a conversation. Open it when you push your first commit. Comment, ask for feedback, commit to other people’s branches if that helps them out. The pull request is the story of how the feature came into being.
  • Integrate Frequently
    Pull master into your branch regularly, to avoid difficult merges.
  • Pair on Merges
    Pair program on difficult merge conflicts, preferably with the person who wrote the code you are merging with. (Pair programming in general is highly recommended, but off topic here.)
  • Review Tests Only
    If you use TDD, and you are short on time, review only the tests. If these are satisfactory, the implementation will probably be ok too.
  • Solve the Problem Before Coding
    Catching problems early when the code is written is great, but catching them before the code is written, is better. Go to the whiteboard with a couple of team members and draw out a solution everybody agrees with. Reviewing the code afterwards will be easier because you already have a model on the whiteboard.

I’m assuming in this post that you use GitHub, but there are other solutions, such as ReviewBoard, BarKeep, Phabricator, and Gerrit. I did a little bit of research last year, and felt that GitHub was the best tool, but I have no hands-on experience with the others. YMMV. The important factor for me is that you can review stories or branches as a whole, and comment inline. (Update: @tvlooy suggested GitLab.)

Pitfalls and anti-patterns

  • Merge Buddies
    As soon as you impose rules, you’ll find people who try to game the system. Sometimes two team members consistently merge each other’s pull requests, without actually looking at them. “I’ll merge yours if you merge mine.” They appear to conform to the rules, but of course it’s just a variation on their old habit of pushing to master. Catch this early. Gatekeeper can be a good remedy.
  • GateKeeper
    One person is responsible for doing all the reviews and the merges. It’s not necessarily an anti-pattern, as it can be useful to prevent people from cheating. But if you keep having a long term Gatekeeper, you have serious problems. People will not feel responsible for their code, let alone others. They will not learn from other people’s code, and they will adapt their code to the Gatekeeper’s comments, without bothering to understand why. Avoid when possible.
  • Total Lockdown
    One step further than having a Gatekeeper, is taking away everybody’s permissions to touch the main repository, and forcing them to use their own forks and send pull requests from there. It an be a good measure if the situation is really problematic, but it’s a terrible downer for morale. Deserved or not, people will feel distrusted. (Update: To clarify, it’s not a problem if they never had pull permissions from the beginning.)
  • The Apologists
    “We don’t have time for code reviews.” Or: “Code reviews do not apply in this project.” As with all discussions about software quality, there will always be people who feel it’s perfectly alright to save time writing junk first, and then waste huge amounts of it bugfixing and putting out fires. They probably don’t write tests either, or refactor code. This is a deeper issue that won’t be fixed by enforcing code reviews alone.
  • Cowboys and Hermits
    Some people don’t want their code looked at. They write their share, and guard it from others, by making sure they are the only ones working on that area. That’s unacceptable, and you’ll need to figure out why they act this way.
  • Issue Trackers
    Many teams discuss issues inside their issue trackers. The distance from the actual code is too great. GitHub’s inline commenting, on individual lines, allows developers to talk about actual code instead of abstract descriptions of code.
  • Focus on Junk Details
    If your code reviews focus on having the correct amount of spaces in the indentation, or typos in comments, you are wasting the effort. Focus on the important issues: Are the tests sufficiently conclusive? Is the naming expressive? Is there coupling where none should be? Will we still understand the code 12 months from now? Is it refactorable? …

As usual, I will keep adding to this post as I find more patterns. Got tips?

Update: Apparently Phil Haack blogged about code reviews almost at the exact same time. He has some great tips like keeping a checklist, focusing on the code and not the author, and stepping through the code. Worth reading!