Improve migration docs

[Peter-Darton-i2]

The existing v11 -> v12 migration documentation is missing some necessary information.
The net effect is that, if one follows the documented migration process as-is, it doesn't work.

• step 2 needs to pack before it can publish
• the migration tool doesn't adjust the orb-publish build instructions within config.yml and test-deploy.yml so this needs to be documented instead
• step2 parameters were wrong (see also chore: update keys for parameters step2_test-deploy.yml #216 which also contains a fix for this)

This PR, together with #229 and #230, fixes that.
... and also makes this orb's test-deploy (step2) build "practise what it preaches" by avoiding the dev-publish step for forked-PR builds (as they should fail to get a context).

[KyleTryon]

A little late here but if you are willing to rebase, we have got the examples update so we can safely remove them from this PR. Thanks for the help!

[Peter-Darton-i2]

I've rebased the PR as requested.

Note: You can safely enable the CircleCI -> Project settings -> Advanced "Build forked PRs" flag for this PR ... and once this is merged, you can safely leave it enabled because this PR turns off the dev-publish job (which requires a context) for forked PRs.
i.e. once this is "in" you can let CircleCI do the builds that GitHub has been told to insist upon.

(and if you update other orbs' test-deploy.yml files to follow this pattern, you can then enable forked-PR builds for those too, which will really help non-CircleCI folks contribute usefully to the orb code and generally make your job a little easier 😉)

PS. ensure that the "Pass secrets to builds from forked pull requests" flag is OFF (it defaults to "on"). That setting should never be "on" for a publicly-visible repo.

[KyleTryon]

Highly appreciate the help. Unfortunately it was not (only) the publish job that forces us to disable forked builds. We may still experiment with turning it back on, but the original reason was due to crypto miners. We used to get a LOT of spam PRs that totally changed the config to mine crypto. We actually have a config policy feature that I haven't yet to dove too deeply into which can probably help too.

[Peter-Darton-i2]

Crypto miners? Wow - that's "unsociable" ... but I guess it's not entirely unexpected; any loophole will be exploited if it can be.

I've been told that OPA config policies are "the CircleCI answer" to all of those security concerns ... although I've personally found that OPA syntax rather difficult; if you do find an answer then please do share because we want to have some of our own CircleCI builds be public and forkable too...

[KyleTryon]

Please remove all changes to the CI process to keep this a docs only change. Was there a reason for these config changes?

[Peter-Darton-i2]

The reasons are two-fold.

Improved example of how to build an orb

This orb is the orb that builds orbs; it is "the primary source of truth" when it comes to building orbs.
As a result, the documentation here (especially the example code) in this orb contains the most "official" recommendation for how to build an orb.

This PR updates the example orb-build code such that CI builds from forked-PRs don't try to do a dev-publish (as they wouldn't have access to the context, e.g. as demonstrated by the failure in this orb PR).
i.e. this makes the orb build process suitable for use "in the more general usecase".

This change is an essential step on the road towards enabling forked-PR builds on CircleCI's public orbs.
While I accept that this step isn't the only thing that'll need to change (as per your previous comments about malicious PRs), it is the only thing that will need to change within this orb's source code - all the other concerns will be solved by changes in other places.

i.e. this change to "how we should all be building orbs" is "the final link in the chain" towards having viable public CI builds of forked-PRs on orb code.

Dogfooding

aka "Eating your own dog food" aka "Drinking our own champagne".
If we're going to tell everyone else to do this, we should "Put our money where our mouth is" and do it ourselves.

... and because this orb is, itself, an orb, it should follow its own recommendations (otherwise we'll look insincere and hypocritical).

In summary

We should all be doing orb builds in a forked-PR-friendly manner, including CircleCI's orb-tools orb.

[KyleTryon]

While I agree with the changes shown here should be added, we can not accept them as part of the same PR that makes changes to the docs, as we are going to be required to squash the commits and rename them using our conventional commit standard.

I recommend we break this up into at least two separate PRs, one for the migration guide, and one for the CI and usage example changes.

[KyleTryon]

I am not sure we need to bake in the dev orb publishing process by default. It may be good for documentation but I'm worried it could also complicate the example as it is not a required step.

[Peter-Darton-i2]

I guess it depends what you mean by "required".
I mean, strictly speaking, folks could code an orb by doing cat | curl -X POST ... and then typing really carefully. It wouldn't be an efficient coding method ... but it is possible.
Similarly, folks don't need to run shellcheck, bats etc; it's not "required" but it is very much recommended.
A "normal" and productive orb development method is to commit to a PR branch and make use of the dev-published version of the orb it generates.
e.g. Most of the time I'm writing orb code, it's for a specific usecase where I've got a build (somewhere else) that needs "the thing I'm writing in the orb", and so after I've done a dev orb publish, I'll go tell my other build to use that dev orb "to prove it works in the real world".
It's incredibly rare that I'll go straight from "build was green" to "release an official version", so it's very rare that I won't want a dev publish, and if I really didn't want one, I could always comment out that bit of the build in my PR. Commenting out code is a lot easier than adding it.

IMO, if we're only going to have one single usage example, that example ought to be "an example of best practice" ... and IMO most users will want dev publishes (not on forked PRs of course, but on internal ones).

So, while I agree that it does make things a little more complicated, if we remove it then either "just about everyone" will need to manually re-add it or they'll end up developing orbs without access to this almost indispensable feature and find that they're making their development experience needlessly hard.

TL;DR: I understand your concerns ... but I disagree 😉

[KyleTryon]

While I agree with the changes shown here should be added, we can not accept them as part of the same PR that makes changes to the docs, as we are going to be required to squash the commits and rename them using our conventional commit standard.

I recommend we break this up into at least two separate PRs, one for the migration guide, and one for the CI and usage example changes.

[Peter-Darton-i2]

OK, understood.
I'll split things up ... although y'all have to merge "both together" or "neither" in order to avoid inconsistencies. As is, this PR keeps everything nice and consistent, ensuring that the orb itself builds using code that's really similar to the example code.

(Sorry for the delay in responding; had an end-of-year work-rush, followed by xmas break. I'll get on with this as soon as I've cleared the holiday backlog)

[Peter-Darton-i2]

I've split this PR up. See #229 and #230.

• This one (Improve migration docs #220) just updates the migration documentation ... although the new documentation does point people at the example & build code from this orb which are updated in Improve migration docs - example #229 and Improve migration docs - build code #230.
• Improve migration docs - example #229 updates the example.
• Improve migration docs - build code #230 updates this orb's build code so that it builds the way it's telling everyone else to do it.