PnP PowerShell and more...

Here I occasionally post about Microsoft 365 Patterns and Practices in general and PnP PowerShell more specifically.

Composing the ultimate Pull Request for PnP

2018-03-30 8 min read Microsoft 365 Patterns and Practices SharePoint

You found an issue in one of the PnP Open Source libraries and you decided to spend some time fixing it. Great! We love your input!

However, we maintain the code base in our free time. Result: there will be a delay before we can look at your submission. To give you an idea, this is the work we do to get your code in to the library, as an example I will take the PnP Sites Core repository:

  1. You write updates, your create a PR
  2. We see that PR coming in. Notice that we get automatically notified of this. No need to highlight this to us through mail/social media/chat/etc. (yes, that happens :-))
  3. Every Friday afternoon (CET) we come together in a meeting and discuss the outstanding PRs, and we assign the PR to a person in the team. You will get notified of this automatically as we make that change on your PR.
  4. That person will evaluate your code. First by simply looking at it. If it’s a very small change (like changing a typo) we can simply decide to merge it as is. But more often we create a separate local branch and merge your changes into that branch. We then start to look at the changes your made in details and run tests, check scenarios etc.

The fact that we merge your branch in a local branch we require you to keep your branch around. E.g. do not create your changes in the dev branch, but create a new branch based upon the dev branch, code your changes, and then submit that branch as a PR. Then keep that branch around until we merged it. See here for more information.

Use Feature Branches

It is very important that you use a feature branch for your PR. E.g. do not commit things to the dev branch in your fork, but create a new branch and put your commits in that one. Then push that branch as a pull request. Keep that branch around until we merged it.

Important: if you submit more PRs make sure to create a feature branch off your dev branch for every feature branch. So do not create a structure like this:

dev > yourfeaturebranch1 > yourfeaturebranch2

This causes merge chaos for us. Instead do:

dev > yourfeaturebranch1
dev > yourfeaturebranch2

We make changes all the time to the main dev branch. It is important that you keep your feature branch up to date with those changes. Merging your branch might be complex due to conflicts (GitHub will notify you of that nowadays) so it’s important that you keep your feature branch in sync with the dev branch you want to merge into. Resolve as many conflicts as you can yourself. #Keep it simple and short This last step can be -greatly- improved if you create atomic PRs and commits. E.g. try to keep your PRs and commits as small as possible. Do not work for weeks on an update, modifying a lot of files, and then commit them as one and put them in a PR. It’s close to impossible for us to correctly evaluate what you have been doing. It can be totally magic what you created, but it can also be something that fully breaks the code… Result: we will postpone your PR or, or worse, not merge it.

Also be as descriptive as possible in your PR. Write down what you did, why you did it, how you tested it yourself, what it solves.

For instance, say you want to change or introduce multiple methods. Make sure to make a separate commit for each method. Then consider that if the changes are atomic to create a few separate PRs instead.

Some submission guidelines

Write your code for SharePoint 2013, SharePoint 2016, SharePoint 2019 and SharePoint Online

We have a single code base that targets 3 different versions of SharePoint. Make sure that your code compiles for those platforms (you can switch build configurations from Debug (= SharePoint Online) to Debug15 (=SharePoint 2013) and Debug16 (= SharePoint 2016) or Debug19 (= SharePoint 2019)).

If your code uses CSOM calls that are not supported by older versions of SharePoint, wrap your lines in compiler directives:

#if !ONPREMISES
// this only works in SPO
#else
// this also works on-premises
#endif

Do not break the SDK

Do not, and I will repeat: do not, change a method signature. People are using the code. Over 8 billion times a month (current as of March 2018) to be precise. If you need a method signature change, these are the ways to do it:

old

public string GetValue(string input)
{ ... }

new

public string GetValue(string input)
{ 
    return GetValue(input, null);
}

// Your new method
public string GetValue(string input, string anotherinput)
{
}

Be careful with deprecation

We have a pattern to obsolete things in PnP Sites Core. You’ll notice that every class is a partial class. Deprecated methods are taken out of the main class and put in another partial class. Say you want to obsolete a method from the Extensions\BrandingExtensions.cs class. Take the method out of the file, and put it in Extensions\Deprecated\BrandingExtensions.deprecated.cs.

We will keep obsoleted methods around for at least 3 months from the release of that branch. So if you obsolete something in April, it will be published in the May release and it will be removed in the August release. Describe that in the *.deprecated.cs file.

Also realize that we are hesitant with deprecation. You not agreeing with a method is -not- a reason for obsoletion. Please don’t let personal opinion come into play here. Also, describe in as much detail as possible why you want to obsolete a method in the PR description.

If you obsolete code, point the developer to what to use instead:

 [Obsolete("Use DisableResponsiveUI(site)")]

##Code has no opinion Opinions, statements, etc. have no place in code. Do not put anything that states a personal opinion in the code and/or remarks.

// This code is cr*ap because of this and that

Will not be merged.

Realize that most people will not use the actual source code but only the compiled version of the library

This means that your method has to be descriptive. Always, add a remarks section on top of a public method:

/// <summary>
/// Disables the Responsive UI on a Classic SharePoint Site
/// </summary>
/// <param name="site">The Site to disable the Responsive UI on</param>
public static void DisableReponsiveUI(this Site site)
{...}

E.g. describe the method, and describe each parameter name.

The PnP Provisioning Schema

This one deserves a specific header. The PnP Provisioning Engine is most likely one of the most popular pieces of open source code that the PnP Effort has produced. Structural in the working of the PnP Provisioning Engine are the templates. They are XML based (at least, most of them, you can if you want use JSON) and we use a Xml Schema file to validate them in the engine to process them. This means, that if you want to add some functionality to the PnP Provisioning Engine (like a completely new section or something), you will -also- have to make a change to the PnP Provisioning Schema. But before you start to code, sit back, think it through, and consider first how you would represent your change in the template.

Then understand that we have a separate repository for the PnP Provsinioning Schema. That’s where you start (and again, don’t start coding yet). The flow is as follows:

Schema Change/Update request -> Approval -> Actual schema update -> Engine implementation can start.

  1. Propose a change to the PnP Provisioning Schema by creating an entry in the correct repository : https://github.com/sharepoint/pnp-provisioning-schema/issues. Describe your change carefully, describe what you want to add/change, what the ideas behind it are, etc. But as detailed as possible
  2. As with the other repositories, we will automatically be notified of the fact that you created an entry. We will have a look at it and we will discuss it. If we believe it’s a functional change that adds something for the majority of people using the Engine we will modify the schema accordingly for the next release. Notice that schema changes do not happen on a monthly basis. Until now we released a new schema approximately every 6 months.
  3. The moment the schema has been implemented one can start coding. This means that the corresponding object model classes (if any) need to be made and the corresponding objecthandler needs to be created/modified. Also, in case of complex scenarios, a specific serialization/deserialization class needs to be created.

Realize that engine changes are complex. We create a lot (understatement) of code to make it work, and the serialization/deserialization part of the engine is a complex beast.

If you feel something is missing (like a property), just go ahead and create an issue in the PnP Provisioning Schema repository. We will pick it up and can build the implementation.

© 2024 by Erwin van Hunen