Version and simple manifest

[mrb]

/c @brynary

[pbrisbin]

I find this a little confusing. What am I specifying here?

• I work with exactly and only 0.0.1
• I work with versions up to 0.0.1
• I work with versions at least 0.0.1
• Something else?

I think we need to support a limited concept of version bounds here. I don't want to sign up for crazy bounds checking logic, but we need to allow the authors to clearly show if it's newer-than, older-than, or exactly the spec_version they're specifying.

[mrb]

@pbrisbin Cool, which one do you think we should do? I don't have strong opinions here.

[pbrisbin]

If we're intending to follow semantic versioning for the spec (which I'm in favor of), then I might suggest the following:

Keep the format as what you have, a single field with a single version value.

Interpret that value like the ~> operator in gemspecs.

So:

spec_version: 0.0.1 means
  I (claim to) support >= 0.0.1 && < 0.1.0

spec_version: 0.1 means
  I support >= 0.1.0 && < 1.0.0

spec_version: 1 means
  I support >= 1.0.0 && < *

(I think I have that right, correct me if I'm wrong)

WDYT?

[brynary]

I actually think this should be a fixed version. It is the version that you are certifying your engine conforms to. If we have another, newer, spec version, it is incumbent on us to determine whether it is backwards compatible.

[pbrisbin]

I don't fully understand what you're saying, but if you can picture a future where engines specify an exact spec version they conform to and we don't end up in dependency hell as the spec evolves and cli/builder start checking and relying on said conformance, then 👍 from me.

[pbrisbin]

We can actually play this out using the include_path feature as an example:

• Pretend it's a few weeks ago, before we built the include_paths feature
• Pretend at that time the spec was v0.1
• Pretend we had this engines.json thing and all engines have spec_version: 0.1
• We implement include_paths and deprecate exclude_paths in SPEC
• What happens?
• We actually want to stop supporting exclude_paths in CLI/Builder
• What happens?

[brynary]

The answer to the first "What happens?" is:

• We bump the spec version to 1.0 (not backwards compatible). The Code Climate CLI, during that time, supports both the 1.x and 0.x SPEC. When it runs an engine, it looks at the declared spec_version, and if it's 0.x it runs it with the old engine harness (probably emitting a warning)

The answer to the second "What happens?" is that we remove the 0.x engine harness from the CLI, and it changes to refuse to run any pre-1.x-designated engines

[pbrisbin]

Cool cool. This is the first I'm hearing of this versioned Engine Harness idea, and it sounds pretty non-trivial -- but OK, that all sounds fine on paper assuming we build said harness.

[wfleming]

@mrb While you're doing this, can you also please add a version or engine_version field as an integer? We're going to need it for cache implementation, so we may as well add it now.

[brynary]

I'd call it engine.json not manifest.json. A manifest is a list of contents, which this includes, but it also includes other things. (In other words, this file contains a manifest, but is not a manifest itself)

[pbrisbin]

Still confused by "the most recent version...", why not just "the version..."?

[mrb]

Good catch

[wfleming]

grammar nit: "an engine.json", not "a engine.json"

[mrb]

Okay, ready for final review @brynary @pbrisbin @wfleming

[brynary]

Looks good. Ship it