| *** tpb has joined #vtr-dev | 00:00 | |
| *** litghost has quit IRC | 07:44 | |
| *** litghost has joined #vtr-dev | 07:52 | |
| acomodi | kem_: I have open some minor PRs: https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/600 and https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/599 | 13:39 |
|---|---|---|
| tpb | Title: netlist: port_bit_name fix by acomodi · Pull Request #600 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 13:39 |
| mithro | Kem_: I'm running a little late today, will be around in 30 minutes and we can do the reformatting | 16:12 |
| mithro | kem_: I'm around now, shall we get started? | 16:43 |
| *** kem_ has joined #vtr-dev | 17:47 | |
| mithro | Heyo kem_! | 17:47 |
| kem_ | mithro: Hey | 17:48 |
| mithro | kem_: I'm just getting libblifparse ready | 17:51 |
| mithro | kem_: Can you take a quick look at https://travis-ci.com/mithro/vtr-code-format/builds/113128925#L256 ? | 17:58 |
| tpb | Title: Travis CI - Test and Deploy with Confidence (at travis-ci.com) | 17:58 |
| mithro | kem_: That is what the error on travis will look like | 17:58 |
| kem_ | mithro: Looks good, perhaps we could echo out a title before the diff, so people would know what it represents | 18:01 |
| mithro | kem_: Okay, let me add that | 18:06 |
| mithro | kem_: How does https://travis-ci.com/mithro/vtr-code-format/builds/113131579 look? | 18:21 |
| tpb | Title: Travis CI - Test and Deploy with Confidence (at travis-ci.com) | 18:21 |
| mithro | kem_: This is what I plan to merge into the vtr repositories -> https://github.com/mithro/vtr-code-format | 18:21 |
| tpb | Title: GitHub - mithro/vtr-code-format (at github.com) | 18:21 |
| mithro | brb - grabbing a coffee | 18:22 |
| kem_ | mithro: Yep looks better | 18:23 |
| kem_ | mithro: A couple of grammar nitpicks: 'You must make the following changes to match the formatting style', and 'The following files must be reformatted!' | 18:25 |
| mithro | kem_: Can you go through the clang-format file at https://github.com/mithro/vtr-code-format/blob/master/.clang-format and check it matches what you are after? | 18:35 |
| tpb | Title: vtr-code-format/.clang-format at master · mithro/vtr-code-format · GitHub (at github.com) | 18:35 |
| mithro | kem_: https://clang.llvm.org/docs/ClangFormatStyleOptions.html | 18:36 |
| tpb | Title: Clang-Format Style Options Clang 9 documentation (at clang.llvm.org) | 18:36 |
| kem_ | mithro: I'll take a look | 18:39 |
| kem_ | mithro: I think AlignAfterOpenBracket should be 'Align' | 18:39 |
| mithro | I'll be back in 5, actually getting that coffee now | 18:41 |
| mithro | kem_: Record which style changes you want in the doc here -> https://docs.google.com/document/d/1h4433ZyX9NpdETBofpZNdP-U2q4Rg0J8lf7-CJ7NkiI/edit# | 18:42 |
| tpb | Title: VtR Formatting Plan - Google Docs (at docs.google.com) | 18:42 |
| mithro | kem_: I also suggest we use "AccessModifierOffset: -2" | 18:42 |
| kem_ | mithro: Is the style you linked significantly different from the one we looked at a while ago? I was pretty happy with that one. | 18:43 |
| kem_ | mithro: Its probably easier to just look at the resulting formatting and see if there are any issues and then figure out how to fix those | 18:44 |
| mithro | kem_: The code formatting output is here -> https://github.com/antmicro/libblifparse/commit/e99dc3a0cdcdd5a51b2938b05532ab43deb38bac | 18:49 |
| mithro | (back now btw :-) | 18:49 |
| kem_ | mithro: Is that the same style as I looked at here: https://logs.timvideos.us/%23vtr-dev/%23vtr-dev.2019-03-07.log.html | 18:53 |
| mithro | kem_: I believe so, unless I've screwed up... | 18:53 |
| kem_ | mithro: OK just checking | 18:53 |
| mithro | I mean, I don't necessarily say I haven't screwed up :-P | 18:54 |
| kem_ | mithro: OK I think I've stuck the obvious tweaks in the doc | 19:00 |
| mithro | kem_: Okay, give me a moment | 19:00 |
| kem_ | mithro: I'm wondering if it would be best to apply this style to VTR first, that way I can take a look through how it formats everything. | 19:01 |
| mithro | kem_: I can do that | 19:01 |
| kem_ | mithro: VTR has more funky code to look at | 19:01 |
| mithro | kem_: Grammar should be fixed at https://travis-ci.com/mithro/vtr-code-format/builds/113133689 | 19:02 |
| tpb | Title: Travis CI - Test and Deploy with Confidence (at travis-ci.com) | 19:02 |
| mithro | kem_: FYI - This is the script I'm using to do the reformat run -> https://gist.github.com/mithro/9bf7ff31e55791c230f00da93de0da0b | 19:17 |
| tpb | Title: vtr-auto-format.py · GitHub (at gist.github.com) | 19:17 |
| mithro | https://github.com/kmurray/libblifparse/compare/master...mithro:format?expand=1 | 19:18 |
| tpb | Title: Comparing kmurray:master...mithro:format · kmurray/libblifparse · GitHub (at github.com) | 19:18 |
| mithro | kem_: Just running for the whole of vtr now | 19:19 |
| kem_ | mithro: Looks like some of the style changes didn't make it in: 'AlignAfterOpenBrack: Align' 'SpacesBeforeTrailingComments: 1' 'AllowShortBlocksOnASingleLine: true' | 19:25 |
| kem_ | mithro: I'd also probably add 'AllowShortCaseLabelsOnASingleLine: true' | 19:25 |
| mithro | kem_: I haven't added your changes yet... | 19:26 |
| mithro | kem_: https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/608/files | 19:44 |
| tpb | Title: Automated reformat the vpr code base by mithro · Pull Request #608 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 19:44 |
| mithro | kem_: Just adding your suggested style changes now... | 19:46 |
| mithro | kem_: You still here? | 20:17 |
| kem_ | mithro: Yep just review the diff with the latest changes | 20:18 |
| mithro | kem_: The changes to the clang config should be in the pull request now | 20:21 |
| mithro | kem_: I'm just going to run down stairs and grab some takeaway lunch, be back in 5 | 20:22 |
| kem_ | Some preprocessor indenting was lost: https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/608/files#diff-59c8309bb5e408c01166639ecbc6f2b5R50 | 20:25 |
| tpb | Title: Automated reformat the vpr code base by mithro · Pull Request #608 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 20:25 |
| kem_ | Looks like 'IndentPPDirectives: AfterHash' would fix it | 20:25 |
| kem_ | https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/608/files#diff-31ec6c191fb261c72ad6804a23bf1a26R219 | 20:33 |
| tpb | Title: Automated reformat the vpr code base by mithro · Pull Request #608 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 20:33 |
| kem_ | Looks like 'AllowShortIfStatementsOnASingleLine: WithoutElse' would fix it | 20:33 |
| kem_ | For https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/608/files#diff-31ec6c191fb261c72ad6804a23bf1a26R263 'AllowShortCaseLabelsOnASingleLine: true' | 20:34 |
| tpb | Title: Automated reformat the vpr code base by mithro · Pull Request #608 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 20:34 |
| mithro | kem_: Looks like WithoutElse is newer than Clang-Format-5.0 | 20:37 |
| mithro | I'm not sure how to tell when an option was added.... | 20:40 |
| mithro | Looks like -> https://releases.llvm.org/6.0.0/tools/clang/docs/ClangFormatStyleOptions.html# | 20:41 |
| mithro | kem_: Looks like WithoutElse is only in the unreleased clang-9.0 :-( | 20:42 |
| kem_ | mithro: OK, how about just setting it to true? | 20:42 |
| mithro | kem_: Testing... | 20:43 |
| mithro | kem_: Look now? | 20:47 |
| mithro | kem_: This is what you are after, right? -> https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/608/files#diff-31ec6c191fb261c72ad6804a23bf1a26R218 | 20:48 |
| tpb | Title: Automated reformat the vpr code base by mithro · Pull Request #608 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 20:48 |
| kem_ | mithro: Yep! | 20:48 |
| mithro | kem_: Need to remember to push the config before I try and merge it -- works better :-P | 20:49 |
| mithro | kem_: I think I have all your suggested changes to the formatting options in now... | 20:53 |
| kem_ | mithro: The only last issue I see is: https://github.com/verilog-to-routing/vtr-verilog-to-routing/pull/608/files#diff-4ac57f609972e5b875fd99f90d6f518eR708 | 20:55 |
| tpb | Title: Automated reformat the vpr code base by mithro · Pull Request #608 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 20:55 |
| kem_ | mithro: Not sure why its collapsing that way since it did the right thing elsewhere | 20:56 |
| mithro | kem_: How do you want that wrapped? | 20:56 |
| kem_ | mithro: I'd expect the line-breaks to be respected but aligned to the open bracket | 20:59 |
| mithro | kem_: I think it is caused by "ColumnLimit: 0" | 20:59 |
| mithro | It also might be BinPackParameters | 21:02 |
| mithro | kem_: Might I also suggest `IndentCaseLabels: false`? | 21:05 |
| kem_ | mithro: OK I'm happy with things as is. It looks like if it is formatted consistently first (which that example was not) clang format does the right thing. It just means some manual one-time fix-ups are required in those cases. | 21:48 |
| kem_ | mithro: I'm thinking to merge the formatting scripts etc and then do the format myself so I can do some of those fix-ups. | 21:49 |
| mithro | kem_: give me 10m | 21:52 |
| mithro | kem_: Still here? | 22:13 |
| kem_ | mithro: Yep | 22:13 |
| mithro | kem_: Okay, here is what I suggest we do... | 22:14 |
| mithro | kem_: "git clone https://gist.github.com/mithro/9bf7ff31e55791c230f00da93de0da0b vtr-auto-format" | 22:15 |
| tpb | Title: vtr-auto-format.py · GitHub (at gist.github.com) | 22:15 |
| kem_ | mithro: OK | 22:16 |
| mithro | kem_: Go fork https://github.com/mithro/vtr-code-format into your user | 22:16 |
| tpb | Title: GitHub - mithro/vtr-code-format (at github.com) | 22:16 |
| kem_ | mithro: OK | 22:17 |
| mithro | Add the following lines to your `~/.gitconfig` file | 22:17 |
| mithro | [rerere] | 22:17 |
| mithro | enabled = true | 22:17 |
| mithro | kem_: Make sure you have clang-format-7.0 installed | 22:17 |
| kem_ | mithro: Yep | 22:18 |
| mithro | kem_: Then for the thing you want to auto-format, fork it to under your user | 22:18 |
| kem_ | mithro: OK... where is this going? | 22:20 |
| mithro | kem_: then the process to generate an auto-format branch is; "cd vtr-verilog-to-routing; ~/github/kmurray/vtr-auto-format/vtr-auto-format.py --user kmurray" -- that should force push a `format` branch to https://github.com/kmurray/vtr-verilog-to-routing repository | 22:22 |
| tpb | Title: GitHub - verilog-to-routing/vtr-verilog-to-routing: Verilog to Routing -- Open Source CAD Flow for FPGA Research (at github.com) | 22:22 |
| mithro | kem_: The script does; | 22:23 |
| mithro | (a) git merge git+ssh://github.com/kmurray/vtr-code-format.git | 22:23 |
| mithro | (b) Let you fix the merge conflicts (which rerere should then save) | 22:23 |
| mithro | (c) Run the format command | 22:23 |
| mithro | (d) Commit it | 22:23 |
| mithro | (e) Add a commit telling git hyper-blame to ignore the formatting commit | 22:24 |
| mithro | (f) push the changes to the `format` branch of your github repository | 22:24 |
| mithro | kem_: Be warned that it resets things to `origin/master` before running | 22:24 |
| mithro | kem_: The idea is that you keep fixing the settings in the vtr-code-format repository and then rerun the vtr-auto-format command which regenerates the output based on that... | 22:25 |
| kem_ | mithro: OK, I'll give it a try | 22:27 |
| mithro | kem_: If something goes wrong, I'm here to help | 22:30 |
| mithro | I'm just trying to figure out the best way to make sure everyone has a copy of clang-format-7 | 22:30 |
| mithro | kem_: What developer environments do you support? | 22:31 |
| kem_ | mithro: Mostly ubuntu, these days typically 16.04 and 18.04. Although we still build and test on 14.04 | 22:31 |
| mithro | kem_: Okay | 22:32 |
| mithro | kem_: The author for the formatting changes should be "VtR Robot <[email protected]>" | 22:34 |
| mithro | kem_: How's it going? | 22:56 |
| mithro | kem_: I just looking at a good way to get people a clang-format version... | 23:25 |
Generated by irclog2html.py 2.13.1 by Marius Gedminas - find it at mg.pov.lt!