Nanoc 4 upgrade guide: some pitfalls

[th-h]

I have - very much belatedly - upgraded my site to Nanoc 4, using your outstanding upgrade guide, mostly without difficulties - but it took me a disproportionately long time to figure out two changes:

• Identifiers are no longer strings, so using string functions like item.identifier.split will not work anymore. That is indeed covered in the "Troubleshooting" section of the upgrade guide, but it escaped me, so I had to google and search through the Nanoc group on Google Groups for hours. :-) I would suggest to point that out much more prominently, perhaps even in the "Quick upgrade guide".

• If the third argument to #new_item or #new_layout is a string, it will be converted to a full identifier. When using your own data source, it's not enough to switch to #new_item from Nanoc::Item.new and add identifier_type: legacy to the configuration (which will have no effect at all) - you have to use a Nanoc::Identifier instance of the legacy type for #new_item. That's not mentioned at all, as far as I can see; I had to piece that together from the source, the Data sources documentation and some examples from the Identifiers and patterns docs.

Perhaps the upgrade guide could be improved still further.

[denisdefreyne]

Some rough thoughts:

Identifiers are no longer strings, so using string functions […] will not work anymore.

It might be worth adding commonly-used functions from String and adding them onto Nanoc::Identifier. This would make #split usable on a Nanoc::Identifier. (On the other hand, .split('/') is something that is probably undesirable—use Nanoc::Identifier#components instead.)

The Troubleshooting section could explicitly mention “Undefined method … on Nanoc::Identifier” so that it is easier to search for.

If the third argument to #new_item or #new_layout is a string, it will be converted to a full identifier.

This is indeed icky, especially because I believe it to be possible to detect whether a string represents a full or legacy identifier (if it ends with a slash, it’s legacy, otherwise it is not).

[denisdefreyne]

Another thought: Nanoc::Identifier.new('/foo/') should raise an error, because '/foo/' is not a valid full identifier (it can’t end with a slash).

[th-h]

(On the other hand, .split('/') is something that is probably undesirable—use Nanoc::Identifier#components instead.)

Ah! Just learned something about Nanoc again. :-)

This is indeed icky, especially because I believe it to be possible to detect whether a string represents a full or legacy identifier (if it ends with a slash, it’s legacy, otherwise it is not).

Sounds like a good idea!

Another thought: Nanoc::Identifier.new('/foo/') should raise an error, because '/foo/' is not a valid full identifier (it can’t end with a slash).

I didn't try the example from the guide - I encountered that problem in one of my sites.

[denisdefreyne]

Another thought: Nanoc::Identifier.new('/foo/') should raise an error, because '/foo/' is not a valid full identifier (it can’t end with a slash).

I didn't try the example from the guide - I encountered that problem in one of my sites.

What I meant to say is that Nanoc::Identifier.new('/foo/') ought to raise an error—it currently doesn’t. I’m working on a PR for this now.

[denisdefreyne]

Here’s the PR that will raise an error when constructing invalid full identifiers: nanoc/nanoc/pull/1206

[denisdefreyne]

And more documentation for the new error: #204

[denisdefreyne]

@th-h I believe that this issue can now be closed. Can you verify that I’ve tackled all the problems mentioned in this issue?

[th-h]

Yes, I think you got everything. (And most people will have upgraded to 4.x long before :-)).

I cross-checked the updated upgrade guide with the entry in my (German language) blog about my upgrade experience, and AFAIS all issues I ran into are covered.

So I think the issue can be closed now.

[denisdefreyne]

Excellent! Thanks for your input.