Thanks Paul!

> On Dec 10, 2014, at 3:50 PM, Robinson, Paul 
> <[email protected]> wrote:
> 
> Reverted in r223987.  Sorry about the data lossage!
>   <>
> From: [email protected] 
> [mailto:[email protected]] On Behalf Of Robinson, Paul
> Sent: Wednesday, December 10, 2014 3:47 PM
> To: Chris Matthews
> Cc: Richard Smith; [email protected]
> Subject: RE: [PATCH] Rename preprocessor symbols to be more descriptive. NFC
>  
> No, that was not my intent at all; it was just a "shouldn't this have a 
> better name?" review comment.  Which I originally resisted, then gave in…
> I had no idea anything outside of clang/lib/Basic/Version.cpp depended on it.
> I am perfectly happy to revert r223578 and leave everything the way it was!
> Gimme a second…
> --paulr
>  
>  
> From: Chris Matthews [mailto:[email protected]] 
> Sent: Wednesday, December 10, 2014 3:37 PM
> To: Robinson, Paul
> Cc: Richard Smith; Jordan Rose; [email protected]
> Subject: Re: [PATCH] Rename preprocessor symbols to be more descriptive. NFC
>  
> Hi Paul, 
>  
> This change is causing a problem with the perf data.
>  
> The bots set SVN_REVISION during builds.  LNT then uses the revision that the 
> compiler reports to index the performance data.  With this change, all 
> performance data is being lost (well it is all ending up as 0).  All 
> performance data being collected since this change has been lost on all the 
> bots.
>  
> http://llvm-lnt.herokuapp.com/db_default/v4/nts/recent_activity 
> <http://llvm-lnt.herokuapp.com/db_default/v4/nts/recent_activity>
>  
> We are going to need to change every bot/build to use the new name.  Was it 
> your intention to modify the build’s interface?  If so, could you revert 
> while we coordinate the bot updates?
>  
> Maybe making this not a silent error would help. Something like error if 
> SVN_REVISION is passed and CLANG_REVISION is not.  Then at least we don’t 
> have compilers with no version data encoded in them, when we expect them to.
>  
> Thanks!
>  
> On Dec 5, 2014, at 9:07 PM, Robinson, Paul 
> <[email protected] 
> <mailto:[email protected]>> wrote:
>  
> r223578, thanks!
>  
> From: [email protected] <mailto:[email protected]> [mailto:[email protected] 
> <mailto:[email protected]>] On Behalf Of Richard Smith
> Sent: Friday, December 05, 2014 5:15 PM
> To: Jordan Rose
> Cc: Robinson, Paul; [email protected] <mailto:[email protected]>
> Subject: Re: [PATCH] Rename preprocessor symbols to be more descriptive. NFC
>  
> +1.
>  
> On Fri, Dec 5, 2014 at 4:35 PM, Jordan Rose <[email protected] 
> <mailto:[email protected]>> wrote:
> +1 from me, and I like the name choice.
> 
> > On Dec 5, 2014, at 15:41, Robinson, Paul 
> > <[email protected] 
> > <mailto:[email protected]>> wrote:
> >
> > This seemed a *little* too intricate to commit directly even if it is NFC.
> > Also gives everyone a chance to disagree with the name choices. :-)
> >
> > This is a followup to a GetSVN.cmake change; Sean Silva and Jordan Rose
> > asked to have the name of the SVN_REVISION/SVN_REPOSITORY symbols changed.
> > This requires changing both the Makefile and CMakeLists.txt so they both
> > generate the same symbol name, and Version.cpp which is the only consumer.
> >
> > The header generated by CMakeLists.txt/GetSVN.cmake used to look like this:
> >
> > #define LLVM_REVISION "223538"
> > #define LLVM_REPOSITORY "http://llvm.org/svn/llvm-project/llvm/trunk 
> > <http://llvm.org/svn/llvm-project/llvm/trunk>"
> > #define SVN_REVISION "223538"
> > #define SVN_REPOSITORY "http://llvm.org/svn/llvm-project/cfe/trunk 
> > <http://llvm.org/svn/llvm-project/cfe/trunk>"
> >
> > and now looks like this:
> >
> > #define LLVM_REVISION "223538"
> > #define LLVM_REPOSITORY "http://llvm.org/svn/llvm-project/llvm/trunk 
> > <http://llvm.org/svn/llvm-project/llvm/trunk>"
> > #define CLANG_REVISION "223538"
> > #define CLANG_REPOSITORY "http://llvm.org/svn/llvm-project/cfe/trunk 
> > <http://llvm.org/svn/llvm-project/cfe/trunk>"
> >
> > --paulr
> > <svn_revision.diff>
> 
> 
> _______________________________________________
> cfe-commits mailing list
> [email protected] <mailto:[email protected]>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits 
> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>  
> _______________________________________________
> cfe-commits mailing list
> [email protected] <mailto:[email protected]>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits 
> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to