Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
On Jan 4, 2011, at 8:15 PM, nathan binkert wrote: I think we've beat this to death on the mailing list. My only comment is that before it becomes part of the commit hook, please make sure that you can't throw anything at it that will cause an exception. A better thing to do is handle all exceptions in the hook and just return failure if used as a hook. Right? Yea, although debugging it is ugly then, but yea probably. Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
I think we've beat this to death on the mailing list. My only comment is that before it becomes part of the commit hook, please make sure that you can't throw anything at it that will cause an exception. A better thing to do is handle all exceptions in the hook and just return failure if used as a hook. Right? Yea, although debugging it is ugly then, but yea probably. I can still print the stack trace, but abort the transaction cleanly. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
On Mon, Dec 27, 2010 at 11:56 AM, nathan binkert n...@binkert.org wrote: I like the former idea... just plain README as a filename seems good enough to me. The latter idea (putting it in the root directory with GENERATED in the name) has the problem that if you don't know that you're looking for auto-generated files, you're not likely to realize this file is relevant to your problem. Ok. I can agree there. Would MISSING_FILES be better? My main point is that the location is what matters most. If you're agreeing that putting something in the directories where the generated files might be is the right thing to do? In that case, I don't think that the filename matters that much; README is fine, MISSING_FILES is probably better. Conversely, I also think that no matter what you call the file, having it in the root directory is not going to be as effective. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
My main point is that the location is what matters most. If you're agreeing that putting something in the directories where the generated files might be is the right thing to do? In that case, I don't think that the filename matters that much; README is fine, MISSING_FILES is probably better. I think that's fine. There are other files that people tend to search for as well, no? I was more wondering if there was anything else that we should do. I don't have any great ideas. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
I could go either way on the explicit directory... I think new users do get confused when some headers aren't where they expect them to be with no obvious way of knowing why, so even though I agree that it seems a little inelegant, I'm not opposed to the idea. If it's just a matter of typing, we could call the directory gen instead of generated. I don't think the separate block (w/o being in a separate directory) is a good idea, since that means not only will people get confused when looking for header files, they'll get confused when we tell them their include ordering doesn't follow the style guide too. I.e., we're not adding much info, we're just introducing a new aspect that subtly relies on the same non-obvious info. Maybe a better idea is to create src/debug and src/params and put a single README.GENERATED file in there that says, the files in this directory are autogenerated during the build phase. Though perhaps we should put that in the root directory and try to list all of the files that one might find that are generated. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
On Mon, Dec 27, 2010 at 6:10 AM, nathan binkert n...@binkert.org wrote: Maybe a better idea is to create src/debug and src/params and put a single README.GENERATED file in there that says, the files in this directory are autogenerated during the build phase. Though perhaps we should put that in the root directory and try to list all of the files that one might find that are generated. I like the former idea... just plain README as a filename seems good enough to me. The latter idea (putting it in the root directory with GENERATED in the name) has the problem that if you don't know that you're looking for auto-generated files, you're not likely to realize this file is relevant to your problem. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
I like the former idea... just plain README as a filename seems good enough to me. The latter idea (putting it in the root directory with GENERATED in the name) has the problem that if you don't know that you're looking for auto-generated files, you're not likely to realize this file is relevant to your problem. Ok. I can agree there. Would MISSING_FILES be better? Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
On 2010-12-25 20:10:03, Nathan Binkert wrote: One big question I have is, do people like the sorting order. This went along with what Steve had in his wish list for the perl version, but it isn't as simple as M-x sort-lines anymore. Of course, people could run this on their code. I could even plug it into the style hook. One other question is, should we separate out automatically generated headers? (like those for params and soon, debug flags?) If so, should I rename them at all, put them in a subdirectory (generated/params/Foo.hh), or leave them alone? Given that there is going to be a bunch of churn, we may as well do it all at once. I'll try to add the sorting function to the style hook, but that will be later. I probably need to do a bit of work to make the commit hook work better for some people anyway. Do people have it enabled generally? - Nathan --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/362/#review579 --- On 2010-12-25 20:05:55, Nathan Binkert wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/362/ --- (Updated 2010-12-25 20:05:55) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- util: python implementation of a routine that will sort includes I didn't realize that the perl version existed when I started this, this version has a lot more features than the previous one since it will sort and separate python, system, and m5 headers in separate groups, it will remove duplicates, it will also convert c headers to stl headers Diffs - util/file_types.py PRE-CREATION util/sort-includes f249937228b5 util/sort_includes.py PRE-CREATION Diff: http://reviews.m5sim.org/r/362/diff Testing --- Thanks, Nathan ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
I don't think you should put those headers in a generated directory. That would be more cumbersome to use, and I don't think it would gain much. Gabe Nathan Binkert wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/362/ On December 25th, 2010, 8:10 p.m., *Nathan Binkert* wrote: One big question I have is, do people like the sorting order. This went along with what Steve had in his wish list for the perl version, but it isn't as simple as M-x sort-lines anymore. Of course, people could run this on their code. I could even plug it into the style hook. One other question is, should we separate out automatically generated headers? (like those for params and soon, debug flags?) If so, should I rename them at all, put them in a subdirectory (generated/params/Foo.hh), or leave them alone? Given that there is going to be a bunch of churn, we may as well do it all at once. I'll try to add the sorting function to the style hook, but that will be later. I probably need to do a bit of work to make the commit hook work better for some people anyway. Do people have it enabled generally? - Nathan On December 25th, 2010, 8:05 p.m., Nathan Binkert wrote: Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Nathan Binkert. /Updated 2010-12-25 20:05:55/ Description util: python implementation of a routine that will sort includes I didn't realize that the perl version existed when I started this, this version has a lot more features than the previous one since it will sort and separate python, system, and m5 headers in separate groups, it will remove duplicates, it will also convert c headers to stl headers Diffs * util/file_types.py (PRE-CREATION) * util/sort-includes (f249937228b5) * util/sort_includes.py (PRE-CREATION) View Diff http://reviews.m5sim.org/r/362/diff/ ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
One other question is, should we separate out automatically generated headers? (like those for params and soon, debug flags?) If so, should I rename them at all, put them in a subdirectory (generated/params/Foo.hh), or leave them alone? Given that there is going to be a bunch of churn, we may as well do it all at once. I don't think you should put those headers in a generated directory. That would be more cumbersome to use, and I don't think it would gain much. I think I agree, though the motivation was to indicate to users that these files were generated and that they wouldn't be able to find them. It does lead to confusion some times. I could put them in a separate block which I would lean towards or just sort them in with other m5 headers. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: util: python implementation of a routine that will sort includes
On Sun, Dec 26, 2010 at 10:36 AM, nathan binkert n...@binkert.org wrote: One other question is, should we separate out automatically generated headers? (like those for params and soon, debug flags?) If so, should I rename them at all, put them in a subdirectory (generated/params/Foo.hh), or leave them alone? Given that there is going to be a bunch of churn, we may as well do it all at once. I don't think you should put those headers in a generated directory. That would be more cumbersome to use, and I don't think it would gain much. I think I agree, though the motivation was to indicate to users that these files were generated and that they wouldn't be able to find them. It does lead to confusion some times. I could put them in a separate block which I would lean towards or just sort them in with other m5 headers. I could go either way on the explicit directory... I think new users do get confused when some headers aren't where they expect them to be with no obvious way of knowing why, so even though I agree that it seems a little inelegant, I'm not opposed to the idea. If it's just a matter of typing, we could call the directory gen instead of generated. I don't think the separate block (w/o being in a separate directory) is a good idea, since that means not only will people get confused when looking for header files, they'll get confused when we tell them their include ordering doesn't follow the style guide too. I.e., we're not adding much info, we're just introducing a new aspect that subtly relies on the same non-obvious info. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] Review Request: util: python implementation of a routine that will sort includes
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/362/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- util: python implementation of a routine that will sort includes I didn't realize that the perl version existed when I started this, this version has a lot more features than the previous one since it will sort and separate python, system, and m5 headers in separate groups, it will remove duplicates, it will also convert c headers to stl headers Diffs - util/file_types.py PRE-CREATION util/sort-includes f249937228b5 util/sort_includes.py PRE-CREATION Diff: http://reviews.m5sim.org/r/362/diff Testing --- Thanks, Nathan ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
