treycarroll
Giga Guru

Over the past few years different folks from SN have spoken with my ITSM managers regarding the topic "the upcoming upgrade to (insert release name here)".     During these sessions the SN reps have harped on the fact that "if you touch it you own it", and that we need to "think about copying things rather than modifying them".     After such meetings, my managers have sat in the meetings listening to the number of skips in the upgrade report and frowned, "Why didn't we stick with best practice and avoid customizations?"     Well, um, we didn't stick with that advice because it's silly, worse, it's actually counter productive.

Let's take an example.

We engaged SN professional services to set up our instance and they made some choices to fit our requirements.   One of those choices was to add a Cancelled state (9) to the task table.     Well, if you're going to do that and you want Workflow conditions to work with this new state, you have to make a corresponding update to the "SNC - Run Parent Workflows" Business Rule, since the condition on it OOTB makes it only run on a state change to 3, 4 or 7.    

So now to the point.     If you blindly accept the advice to always avoid updating an OOTB Business Rule, you would make a copy of "SNC - Run Parent Workflows", an exact replica with the exception that the condition says current.state.changesTo(9) instead of 3 or 4 or 7.     So then the same code would now exist in two places (a clear violation of the DRY principle of good software design), and this new code would be disconnected from the original's sys_id.    

Now you could make the argument that the new Business rule could be named in a way that makes it appear adjacent to the original alphabetically in the list view and that a comment could be added to the original, and I would agree, that's pretty good.   However, modifying the original is clearly the better choice.   Here's why:       The upgrade process is designed to show you when you have a collision, when something you have changed is colliding with the OOTB version you "forked".     You want this.     If we look specifically at the "SNC - Run Parent Workflows" example the reason becomes very clear.

Let's assume that we copied rather than modifying the Business Rule.   Now let's assume that in release "Hackensack" Service now decides that they're going to add a cancelled state of 9 to task and an "or state changes to 9" clause onto their OOTB version of "SNC - Run Parent Workflows".       Unfortunately, you, the only person who remembers about the decision to copy this Business Rule, are trekking in Nepal during the upgrade.   The upgrade runs and updates "SNC - Run Parent Workflows", adding the "state changes 9" condition. Everything looks good, and the upgrade rolls to production.   After all, there was no mention of a need to look at this Business Rule in the upgrade logs, so everything went as it should, right?

Well, no.     We just missed the opportunity to see that OOTB should be accepted and that we should delete our Business Rule, thereby keeping our code base free of unneeded legacy code.     Had we customized the original, the upgrade report would have given us the opportunity to review the change alongside our customizations and we would have clearly seen that we could simply click the "revert to OOTB" button and be done.     As things stand, we short circuited the very mechanism that is designed to alert us when we need to review two pieces of code and resolve a conflict and potentially perform a merge.

Let's image a developer working a source code versioning system like GIT, adding an enhancement to a major open source project, the simple arithmetic calculator.  

The project is really going well and they have already developed the ability to add/subtract/multiply/divide integers.     As an astute developer, we notice that the app might be more useful if it could also work with decimals.     So we clone the repository and set out to work, but rather than modifying the "add" function we decide to create another button on the calculator, a "+ dec" (add decimal) button.     Correspondingly, all of the associated code is put into separate "add decimal" objects, all to avoid making changes to the original versions of "add".   (After all, nobody likes doing those painful merges, and we want our pull request to be accepted.)

Now, really, how would a pull request for such code be received by the maintainers of the project?

Merges are a part of life for any real-world code base - maybe not our favorite part - but a necessary, wholesome, healthy part of life in the real world of software development.   It's time we started treating ServiceNow code like real code, subject to the same burdens and responsibilities as the code-base for any other application.   And it's time for ServiceNow developers to help managers understand when they're crossing the lines and making technical decisions, whose implications they don't fully grasp, whose pitfalls they want to avoid.

If the enhancement we're making is an extension of OOTB functionality, it makes sense to modify the original so that we will be guaranteed to be informed when that functionality is changed, so that we can reevaluate our decisions as a part of an upgrade.

3 Comments