API method renaming

[mapsam]

I truly do enjoy giggling at the names of the methods in this API, but it also requires me to consistently refer to the API docs to remember what each method actually does. It seems useful to rename the methods to something more memorable. Proposal:

var sniff = require('@mapbox/mapbox-file-sniff');

sniff.protocol(); // returns protocol string
sniff.type(); // returns file type string
sniff.all(); // returns both object (probably a better name here)

cc @GretaCB @who8mycakes @rclark

[rclark]

I agree that this is a good idea, though I do get to troll you for being a downer.

The other dimension is that there are routines that expect you to hand in a buffer (which you already read from the file), and routines that expect you to provide a file name (and sniffer will read from the file). What do you think about naming in those cases?

[mapsam]

troll you for being a downer

Much deserved! I had a hard time writing this ticket 😢

buffer vs. file

What about detecting a buffer vs string, and doing the work for us?

[mapsam]

Another (more "rip it all apart") option is to only have a single method based straight off the require that returns an object with both pieces of information

var sniffer = require('@mapbox/mapbox-file-sniff');

sniffer('path/to/file.json', function(err, sniffed) {
  console.log(sniffed); // => { protocol: 'omnivore:', type: 'geojson' }
});

[mapsam]

I'm actually feeling pretty good about combining methods into one. Here's some example code in mapbox-upload-validate of needing to use both sniff and quaff methods, which leads to a small instance of callback hell. Not only is it kind of gross, we actually run the same exact code twice since retrieving protocol information requires the actual filetype to be known. Returning both pieces of information seems like a healthy default.

[rclark]

JS methods

method	accept buffer	accept filename	return type	return protocol
sniff	x		x
waft	x			x
quaff		x	x	x

I think that we will ideally retain 2 functions, but they can be renamed. Two functions because we have two different types of input, and it seems worthwhile to avoid the polymorphism IMO. But agree that type/protocol should just be returned from each.

CLI

command	accept buffer	accept filename	return type	return protocol
mapbox-file-protocol		x		x
mapbox-file-type		x	x

The important thing here is that each CLI command needs to return a single string, either the type or the protocol. This prevents us from having to parse outputs in shell scripts. Not sure if anything has to change on the CLI side.

[who8mycakes]

Thanks for bringing this up @mapsam! Renaming is hard, but our future selves will thank us.

[mapsam]

Sounds good @rclark - I'm in favor of keeping two methods, one for each input type. Some spitball ideas:

• sniffer.fromBuffer() & sniffer.fromFile()
• sniffer.buffer() & sniffer.path()
• sniffer.sniffBuffer() & sniffer.sniffPath()

CLI

I had added some flags in the attempt to move to a single method, but that won't work if we have two methods (unless we do some testing within the command itself to figure out buffer/path). Two CLI commands feels overly complex, but happy to not do anything to it for now.

Example single command

$ mapbox-file-sniff path/to/data/file.geojson
# {"protocol":"omnivore:","type":"geojson"}
$ mapbox-file-sniff path/to/data/file.geojson --type
# geojson
$ mapbox-file-sniff path/to/data/file.geojson --protocol
# omnivore:

[rclark]

Sounds good on both fronts. Personally I prefer sniffer.fromBuffer() & sniffer.fromFile() over the other names.

[flippmoke]

@rclark nose a lot how to name a sniffer

[mapsam]

Complete! Final solution was to use fromFile and fromBuffer.

Thanks for the feedback and puns, everyone 😄