You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@thrift.apache.org by Wei Zheng <wz...@hortonworks.com> on 2016/04/18 23:39:54 UTC

Variables name change for irrelevant methods

Hi all,

I made change to an existing .thrift file, by adding a new struct:

struct NewRequest {
    1: required list<i64> txn_ids,
}

After regenerating code, I do see the changes I want. But I also noticed there are a bunch of files which are irrelevant to this change, but have many variable names being changed, for example:

Previously:
                org.apache.thrift.protocol.TMap _map524 = iprot.readMapBegin();
                struct.metadata = new HashMap<Long,MetadataPpdResult>(2*_map524.size);
                long _key525;
                MetadataPpdResult _val526;
                for (int _i527 = 0; _i527 < _map524.size; ++_i527)

Now:
                org.apache.thrift.protocol.TMap _map532 = iprot.readMapBegin();
                struct.metadata = new HashMap<Long,MetadataPpdResult>(2*_map532.size);
                long _key533;
                MetadataPpdResult _val534;
                for (int _i535 = 0; _i535 < _map532.size; ++_i535)

I don't think this kind of change is harmful. But it requires code change for irrelevant files, which is unexpected. Anyone knows the rationale behind this?

Thanks,
Wei

Re: Variables name change for irrelevant methods

Posted by Wei Zheng <wz...@hortonworks.com>.
Thanks Jens,

Your info really helps! Now I can be assured that the irrelevant changes are safe to keep :)

Thanks,
Wei







On 4/19/16, 16:12, "Jens Geyer" <je...@hotmail.com> wrote:

>Hi Wei Zheng,
>
>> Anyone knows the rationale behind this?
>
>There is a built-in mechanism for temporarily variables. It basically relies 
>on a special prefix plus an incremented counter. The numbers are incremented 
>to generate variable names that do not produce collisions.
>
>> I don't think this kind of change is harmful.
>
>Yep, correct. These are all only internal variable names, whose names are 
>only relevant in the given scope.
>
>> But it requires code change for irrelevant files, which is unexpected.
>
>You mean w/regard to your VCS? In theory, generated artifacts are not to be 
>added to the VCS because they can be generated from the IDL. But 
>nevertheless, in real world, one may indeed want to do exactly that, for a 
>number of reasons. But even then, at the end it is generated code, so nobody 
>should really care that much about it. Especially if IDL changes and 
>(generated) code changes are in the same commit.
>
>Have fun,
>JensG
>
>
>
>-----Ursprüngliche Nachricht----- 
>From: Wei Zheng
>Sent: Monday, April 18, 2016 11:39 PM
>To: user@thrift.apache.org
>Subject: Variables name change for irrelevant methods
>
>Hi all,
>
>I made change to an existing .thrift file, by adding a new struct:
>
>struct NewRequest {
>    1: required list<i64> txn_ids,
>}
>
>After regenerating code, I do see the changes I want. But I also noticed 
>there are a bunch of files which are irrelevant to this change, but have 
>many variable names being changed, for example:
>
>Previously:
>                org.apache.thrift.protocol.TMap _map524 = 
>iprot.readMapBegin();
>                struct.metadata = new 
>HashMap<Long,MetadataPpdResult>(2*_map524.size);
>                long _key525;
>                MetadataPpdResult _val526;
>                for (int _i527 = 0; _i527 < _map524.size; ++_i527)
>
>Now:
>                org.apache.thrift.protocol.TMap _map532 = 
>iprot.readMapBegin();
>                struct.metadata = new 
>HashMap<Long,MetadataPpdResult>(2*_map532.size);
>                long _key533;
>                MetadataPpdResult _val534;
>                for (int _i535 = 0; _i535 < _map532.size; ++_i535)
>
>I don't think this kind of change is harmful. But it requires code change 
>for irrelevant files, which is unexpected. Anyone knows the rationale behind 
>this?
>
>Thanks,
>Wei 
>
>

Re: Variables name change for irrelevant methods

Posted by Jens Geyer <je...@hotmail.com>.
Hi Wei Zheng,

> Anyone knows the rationale behind this?

There is a built-in mechanism for temporarily variables. It basically relies 
on a special prefix plus an incremented counter. The numbers are incremented 
to generate variable names that do not produce collisions.

> I don't think this kind of change is harmful.

Yep, correct. These are all only internal variable names, whose names are 
only relevant in the given scope.

> But it requires code change for irrelevant files, which is unexpected.

You mean w/regard to your VCS? In theory, generated artifacts are not to be 
added to the VCS because they can be generated from the IDL. But 
nevertheless, in real world, one may indeed want to do exactly that, for a 
number of reasons. But even then, at the end it is generated code, so nobody 
should really care that much about it. Especially if IDL changes and 
(generated) code changes are in the same commit.

Have fun,
JensG



-----Ursprüngliche Nachricht----- 
From: Wei Zheng
Sent: Monday, April 18, 2016 11:39 PM
To: user@thrift.apache.org
Subject: Variables name change for irrelevant methods

Hi all,

I made change to an existing .thrift file, by adding a new struct:

struct NewRequest {
    1: required list<i64> txn_ids,
}

After regenerating code, I do see the changes I want. But I also noticed 
there are a bunch of files which are irrelevant to this change, but have 
many variable names being changed, for example:

Previously:
                org.apache.thrift.protocol.TMap _map524 = 
iprot.readMapBegin();
                struct.metadata = new 
HashMap<Long,MetadataPpdResult>(2*_map524.size);
                long _key525;
                MetadataPpdResult _val526;
                for (int _i527 = 0; _i527 < _map524.size; ++_i527)

Now:
                org.apache.thrift.protocol.TMap _map532 = 
iprot.readMapBegin();
                struct.metadata = new 
HashMap<Long,MetadataPpdResult>(2*_map532.size);
                long _key533;
                MetadataPpdResult _val534;
                for (int _i535 = 0; _i535 < _map532.size; ++_i535)

I don't think this kind of change is harmful. But it requires code change 
for irrelevant files, which is unexpected. Anyone knows the rationale behind 
this?

Thanks,
Wei