Changed commit.id to commit.id.full

[derrickburns]

In order to make the commit info be convertable to a JSON object, I placed the renamed the commit id to "commit.id.full".

[TheSnoozer]

Hi,
sorry but I don't get the purpose of this pull request.
Why is the commit-info with "commit.id" not convertable to a JSON Object?
Any example use cases which point out a problem?

[derrickburns]

If commit.id represents a JSON object it must be either a primitive, a list, or a map. 

Since it has a string value, then it is a primitive.

commit  : {
  id : "bacd46826abc48cde.."
}

However, commit.id.abbrev is s child of commit.id, implying that commit.id is a map. 

commit : {
   id : {
     abbrev : "bacd46"
   }
}

commit.id cannot be both a primitive and a map at the same time. 

—
Sent from Mailbox

On Sun, Jun 8, 2014 at 7:39 AM, TheSnoozer [email protected]
wrote:

Hi,
sorry but I don't get the purpose of this pull request.
Why is the commit-info with "commit.id" not convertable to a JSON Object?

Any example use cases which point out a problem?

Reply to this email directly or view it on GitHub:
#123 (comment)

[derrickburns]

commit  : {

  id : {

     full: "bacd46826abc48cde.."

     abbrev : "bacd46" 

   }

}
—
Sent from Mailbox

On Sun, Jun 8, 2014 at 8:11 AM, Derrick Burns [email protected]
wrote:

If commit.id represents a JSON object it must be either a primitive, a list, or a map. 
Since it has a string value, then it is a primitive.
commit  : {
  id : "bacd46826abc48cde.."
}
However, commit.id.abbrev is s child of commit.id, implying that commit.id is a map. 
commit : {
   id : {
     abbrev : "bacd46"
   }
}
commit.id cannot be both a primitive and a map at the same time. 
—
Sent from Mailbox
On Sun, Jun 8, 2014 at 7:39 AM, TheSnoozer [email protected]
wrote:

Hi,
sorry but I don't get the purpose of this pull request.
Why is the commit-info with "commit.id" not convertable to a JSON Object?

Any example use cases which point out a problem?

Reply to this email directly or view it on GitHub:
#123 (comment)

[derrickburns]

The issue is that with JSON, the dot notation is used for paths not keys.—
Sent from Mailbox

On Sun, Jun 8, 2014 at 8:14 AM, Derrick Burns [email protected]
wrote:

commit  : {
  id : {
     full: "bacd46826abc48cde.."
     abbrev : "bacd46" 
   }
}
—
Sent from Mailbox
On Sun, Jun 8, 2014 at 8:11 AM, Derrick Burns [email protected]
wrote:

If commit.id represents a JSON object it must be either a primitive, a list, or a map. 
Since it has a string value, then it is a primitive.
commit  : {
  id : "bacd46826abc48cde.."
}
However, commit.id.abbrev is s child of commit.id, implying that commit.id is a map. 
commit : {
   id : {
     abbrev : "bacd46"
   }
}
commit.id cannot be both a primitive and a map at the same time. 
—
Sent from Mailbox
On Sun, Jun 8, 2014 at 7:39 AM, TheSnoozer [email protected]
wrote:

Hi,
sorry but I don't get the purpose of this pull request.
Why is the commit-info with "commit.id" not convertable to a JSON Object?

Any example use cases which point out a problem?

Reply to this email directly or view it on GitHub:
#123 (comment)

[derrickburns]

The JSON file namespace is hierarchical, not flat like the properties file namespace.—
Sent from Mailbox

On Sun, Jun 8, 2014 at 8:32 AM, Derrick Burns [email protected]
wrote:

The issue is that with JSON, the dot notation is used for paths not keys.—
Sent from Mailbox
On Sun, Jun 8, 2014 at 8:14 AM, Derrick Burns [email protected]
wrote:

commit  : {
  id : {
     full: "bacd46826abc48cde.."
     abbrev : "bacd46" 
   }
}
—
Sent from Mailbox
On Sun, Jun 8, 2014 at 8:11 AM, Derrick Burns [email protected]
wrote:

If commit.id represents a JSON object it must be either a primitive, a list, or a map. 
Since it has a string value, then it is a primitive.
commit  : {
  id : "bacd46826abc48cde.."
}
However, commit.id.abbrev is s child of commit.id, implying that commit.id is a map. 
commit : {
   id : {
     abbrev : "bacd46"
   }
}
commit.id cannot be both a primitive and a map at the same time. 
—
Sent from Mailbox
On Sun, Jun 8, 2014 at 7:39 AM, TheSnoozer [email protected]
wrote:

Hi,
sorry but I don't get the purpose of this pull request.
Why is the commit-info with "commit.id" not convertable to a JSON Object?

Any example use cases which point out a problem?

Reply to this email directly or view it on GitHub:
#123 (comment)

[TheSnoozer]

Well ok, now I got the problem.
Renaming the property does not fix that, correct?
Instead the JSON-property-generation should be fixed?!

[derrickburns]

No, renaming commit.id to be commit.id.full resolves the conflict. Then commit.id is a MAP with members full and  abbrev, etc.—
Sent from Mailbox

On Sun, Jun 8, 2014 at 11:11 AM, TheSnoozer [email protected]
wrote:

Well ok, now I got the problem.
Renaming the property does not fix that, correct?

Instead the JSON-property-generation should be fixed?!

Reply to this email directly or view it on GitHub:
#123 (comment)

[derrickburns]

That is what my pull request does.—
Sent from Mailbox

On Sun, Jun 8, 2014 at 11:11 AM, TheSnoozer [email protected]
wrote:

Well ok, now I got the problem.
Renaming the property does not fix that, correct?

Instead the JSON-property-generation should be fixed?!

Reply to this email directly or view it on GitHub:
#123 (comment)

[ktoso]

That's a lot of emails for a small change! ;-)

I agree and see the problem @derrickburns, although I'd like to take into account projects which already use the plugin - this would break them if they rely on the previous key being present.

How are you generating your JSON? Because the usual case is to go via a POJO, like the README.md explains - in which case you have full control over the structure. The other way to generate a JSON file would be to have a git.json file be filtered by maven, with key placeholders (pointing to the plugin's naming).

Anyhow - I'd like to know how you generate your JSON before we merge this

[derrickburns]

Sorry about the barrage of short emails. :)

I read in all properties files and JSON files and map them back to JSON objects.  I use the git-commit-id-plugin feature to generate a property file. 

If you prefer to be backward compatible, you could offer a way to remap the property names before the property file is generated. Then, I could simply provide my own remapping of "commit.id" to "commit.id.full".
—
Sent from Mailbox

On Sun, Jun 8, 2014 at 12:18 PM, Konrad Malawski [email protected]
wrote:

That's a lot of emails for a small change! ;-)
I agree and see the problem @derrickburns, although I'd like to take into account projects which already use the plugin - this would break them if they rely on the previous key being present.
How are you generating your JSON? Because the usual case is to go via a POJO, like the README.md explains - in which case you have full control over the structure. The other way to generate a JSON file would be to have a git.json file be filtered by maven, with key placeholders (pointing to the plugin's naming).

Anyhow - I'd like to know how you generate your JSON before we merge this

Reply to this email directly or view it on GitHub:
#123 (comment)

[ktoso]

I guys,
I agree that this needs addressing one way or another. It's gotten quite late tonight as I was rushing to release an 2.1.12 so I won't make it to release tonight as I'd like this fix to be included... Maybe tomorrow though!

[ktoso]

This refs #122

[ktoso]

Currently I'd argue it's easy enough to not go 1:1 into your json object, you could simply:

def myJsonProps(gitProps: Properties): String = {
  gitProps.put("git.commit.id.full", gitProps.get("git.commit.id"))
  gitProps.remove("git.commit.id")
}

and done, right?

The reason I'm reluctant with this change now is that people are depending on this key in the properties, so we'd break existing users, where as the fix for you is to not directly depend on the props object.

Or even - you can simply write your own json properties file and use maven properties replacement in there - no problem.