Skip to main content

· 4 min read
Alvaro Jose

Over the last few years, some practices appear to be more a dogma than a value adding practice. One of this is Pull Requests.

Why PR's exist

  • Malicious Code Prevention: Pull requests exist mostly as a practice accepted for zero trust environments (ex. Open Source). An attack vector on this type of environment is the ability of anyone to contribute, meaning you could inject code that could create known vulnerabilities that packages will inherit. That is why maintainers validate code from unknown users.

Malicious actors

  • Highly Distributed Teams: PR's can be use for highly distributed teams (around the clock) as a way to do knowledge sharing. If someone in side A of the world can follow and understand the changes for a feature that is being developed in side B of the world.

Distributed Teams

The issue

IS there any value of doing PRs when people work collocated? What is the cost of PRs in trust environments?

The value that normally people give to PRs is the one of having a peer review process. Nevertheless, we will see later in this article that there are less invasive ways to do this.

Some costs of PRs are:

  • Slow Delivery: PRs are a start and stop strategy where there is a gateway to merge code. This is time that needs to be taken by developers (writting & preparing a PR) and reviewers (reviewing, commenting, etc) to go through the process. At the same time is more time the code takes to get to production (merging, re-testing, etc). This is significant for features but also for fixes, meaning you can go from a response time of minutes to hours.
  • Isolation work: When working on branches, devs work on code that works isolated but needs to be merged with a continuous stream of changes. This means that any test isolated will probably be invalidated upon merging.
  • Lack of ownership: As work is done isolated, the developer who creates a PR delegates part or the responsibility to the reviewer. Humans don't have compilers or containers to run the code in our brain, meaning catching errors tends to be out of our realm.
  • Egos: As catching errors tends to be out of our human realm, PRs tend to become a thing related to preferences (Style, patterns, etc). This hardly provides any value to the code as either tools like linters can do this automatically or it brings premature optimizations.
  • Late feedback: Any valid recommendation is actually provided quite late in the process, when the code has already been written and validated. Causing rework that takes time.

The Alternatives

Pull requests are just one of the asynchronous peer code reviews styles. Is not the only way of doing peer reviews.

Some other types of peer reviews that I, personally, like are:

  • Over-the-shoulder: The bases of this is to have a conversation over what has been or is being implemented. This creates a synchronous feedback loop on an async process. It does not fix all the shortcomings of a PR, but it creates a faster feedback loop.
  • Pair Programming / Mob Programming: The idea is that multiple developers work in the same code base in the same computer, creating a synchronous feedback loop in a synchronous process. This with Trunk-based development allows very fast feedback loops at product level, and with the correct tools generates resilience and ownership among developers.

The disclaimer here is I have worked doing pair programming, TDD and trunk-based development for more than 5 years in multiple size companies, and it has always been a bliss.

· 3 min read

As a member of the community that like to generate npm packages like libraries and cli tools, sometimes is difficult to maintain everything and keep your package up to date in the dependencies side. I am a fan of having static dependencies as versioning is not being held correctly in most of the npm world. So if you dont use exact packages you could run in the issue that a broken change makes from the night to the morning your awesome tool to break.

This practice could bring a headache to keep dependencies up to date because is a manual process. And manual process tend to be time consuming (at this point in time I have ~17 npm packages) meaning that if i want to simply do normal maintenance i will have to run everything for all those in maybe weekly or monthly bases.

So is a bit of a no situation for maintainers, but if you dont maintain your package people will not use it, because there is a concern about how active the project is, even if there are no open issues. For solving both of this things what i have decided is to ad to my CI/CD pipeline a script that runs only on cron jobs from travis ci.

os: osx
language: node_js
node_js:
- node
script:
- yarn test:cov
after_success:
- if [[ "${TRAVIS_EVENT_TYPE}" = "cron" ]]; then ./upgrade.sh; fi
deploy:
skip_cleanup: true
provider: npm
email: $NPM_EMAIL
api_key: $NPM_TOKEN
on:
tags: true

as you can see that is the normal .travis.yml for deploying into npm (you will have to define NPM_EMAIL and NPM_TOKEN as enviroment variables in your build configuration), the main diference is the step after success that if its the cron job going will run the next script.

#!/bin/sh

set -e

git config --global user.email $GH_EMAIL
git config --global user.name $GH_USER

git remote add origin-master https://${GH_TOKEN}@github.com/${TRAVIS_REPO_SLUG}.git > /dev/null 2>&1

git fetch origin-master
git checkout -b master-local origin-master/master

yarn upgrade --latest
git add .
git commit --allow-empty -m "updated dependencies [skip ci]"

yarn test
yarn version --patch

git push --quiet origin-master master-local:master
git push --quiet origin-master master-local:master --tags

this script attaches the current state to a branch makes, upgrades the dependencies and if everything works fine generates a new commit and deploy a patch of the packages (you will have to define GH_EMAIL, GH_USER and GH_TOKEN as environment variables in your build configuration).

· 2 min read

I have just finished migrating my static blog from Hexo to Hugo and one of the main things I care about is to be able to do continuous deployment of my profile and blog. There are quite a few blog posts out there but they are based on using shell scripts and it really becomes a pain to give permissions etc. In the next few lines you will see the simplest way I have found to do this (and is currently as this blog post is being published).

You will need to have:

  • A Github account.
  • A Travis CI account.
  • A Github repository with source code of your web page with Hugo (*1)
  • A Github repository with the name <your User or Organization>.github.com (ex. kanekotic.github.com) (*2).
  • A developer token from GitHub with commit capabilities (can create in github Settings -> Developer Settings -> Personal Access Token -> Generate New Token )

I wont cover how to create a Hugo web page as this is best explained in the quick start) of Hugo.

After you are happy with the content of your blog in the repository of source code (*1), and want to start deploying you will need to add a .travis.yml with the next content

sudo: true
dist: trusty

install:
- sudo apt-get --yes install snapd
- sudo snap install hugo

script:
- /snap/bin/hugo

deploy:
provider: pages
local-dir: public
repo: <User or Organization>/<User or Organization>.github.com
target-branch: master
skip-cleanup: true
github-token: $GITHUB_TOKEN
committer-from-gh: true
keep-history: true
on:
branch: master

you will have to change the repo content to match your destination repository (*2). The previous code what does is installs hugo in the deployment machine, builds your web page and deploys using the pages plugin. If you have a custom domain make sure to set the property fqdn to your domain, if not you will overwrite this field in each commit.

After adding the file you will have to go to Travis web page and toggle your code repository (*1) got to More Options -> Settings -> Environment Variables and add GITHUB_TOKEN as the token retrieved from github.

After this in any commit in the master branch of your web page you will get it deployed and go live.

· 2 min read

I have hit a corner case of extension methods and LINQ. Today I was declaring some extension methods to make my code more readable.So I created an extension method that gets an object and performs a direct cast:

public static class GeneralExtensions
{
public static T Cast<T>(this object o)
{
return (T)o;
}
}

The intention was to be able to call my direct castings by something like this:

MyObject.CastTo<MyInterface>();

It happens that in the same namespace I have an extension method that has a LINQ expression

using System;
using System.Collections.Generic;
using System.Linq;

public static class EnumExtenstions
{
public static IEnumerable<string> UseLinq(this IEnumerable<object> collection)
{
return (from object value in collection select value.ToString() ).ToList();
}
}

Adding this first extension method to my code base cause the next error

Error   CS1936  Could not find an implementation of the query pattern for source type 'object'.  'Select' not found.

Having both extension methods in different namespaces (and not referred), or rename Cast<T> to something different solves the issue. This is caused for an overlap of the extension methods where the nearest one to the code is the one called.

##How bad Extension Methods over object could go?

This is an extract from the answer of Eric Lippert, regarding the code:

public static class GeneralExtensions
{
public static T Cast<T>(this object o)
{
return (T)o;
}
}

Side Effects of the cast<T>:

  • Cast<int>(123) unnecessarily boxes the int, (int)123 does not.
  • Cast< short >(123) fails but (short)123 succeeds. There is no conversion from a boxed int to a short.
  • Suppose you have a user-defined conversion from Animal to Shape. Cast<Shape>(new Tiger()) fails but (Shape) new Tiger() succeeds.
  • Suppose q is a nullable int that happens to be null. Cast<string>(q) succeeds! But (string)q would fail at compile time
  • Etc

Cast method has some overlap with the real cast operator but is not a substitute for it. To capture the semantics of the cast operator there is a need to use dynamic, which starts the compiler at runtime and does the compile time analysis on runtime types.

· One min read

I have not experiment to much with fluent interfaces. But is something cool especially to make code that is expressive.

public struct Coordenates
{
public double X { get; set; }
public double Y { get; set; }
public double Z { get; set; }
}

public static class CoordenatesExtensions
{

public static Coordenates X(this Coordenates coordenates, double value)
{
coordenates.X = value;
return coordenates;
}

public static Coordenates Y(this Coordenates coordenates, double value)
{
coordenates.Y = value;
return coordenates;
}
public static Coordenates Z(this Coordenates coordenates, double value)
{
coordenates.Z = value;
return coordenates;
}
}

public class Points
{
private Coordenates point;
public Points()
{
point = new Coordenates().X(2.1).Y(2.4).Z(3.2);
}
}

also can be used with some language properties to make it more expressive

public static class GeneralExtensions
{
public static T As<T>(this object o) where T : class
{
return o as T;
}

public static T Cast<T>(this object o)
{
return (T)o;
}

public static bool Is<T>(this object o)
{
return o is T;
}
}

· One min read

When doing complex objects using an object to help the building is welcome.

public class Complex
{
double x;
double y;
double z;

float height;
float width;

string foreground;
string background;

public Complex()
{
x = 1.456;
y = 1.234;
z = 1.789;

height = 10.12;
width = 10.14;

foreground = "#FFF";
background = "#FA1";
}

}

In this way you remove some complexity of just adding steps in your constructor to something more abstract and can contain the logic.

public class Complex
{
public double X { get; set; }
public double Y { get; set; }
public double Z { get; set; }

public double Height { get; set; }
public double Width { get; set; }

public string Foreground { get; set; }
public string Background { get; set; }

public Complex(ComplexBuildHelper buildHelper)
{
buildHelper.Construct(this);
}

}

public class ComplexBuildHelper
{
public void Construct(Complex reference)
{
BuildPosition(reference);
BuildDimension(reference);
BuildCharacteristics(reference);
}

private void BuildPosition(Complex reference)
{
reference.X = 1.456;
reference.Y = 1.234;
reference.Z = 1.789;
}

private void BuildDimension(Complex reference)
{
reference.Height = 10.12;
reference.Width = 10.14;
}

private void BuildCharacteristics(Complex reference)
{
reference.Foreground = "#FFF";
reference.Background = "#FA1";
}
}