#general
- g
gorgeous-fish-11040
08/21/2015, 3:07 PMFor example at Twitter we use a hidden folder in our monorepo as a folder for caches so users are aware of it. - g
gorgeous-fish-11040
08/21/2015, 3:16 PM@bored-art-40741 what do you think about a separate cache folder for python next to local_artifact_cache? - b
bored-art-40741
08/21/2015, 3:16 PMI like that better, yeah - e
enough-analyst-54434
08/21/2015, 4:24 PMI need to hit a plane in the afternoon so I'll be cutting the release @~1pm Pacific - a
ancient-stone-50795
08/21/2015, 4:54 PMSGTM - m
mammoth-pharmacist-54215
08/21/2015, 5:08 PMif anyone has a spare moment would they mind looking at: https://rbcommons.com/s/twitter/r/2593/ - m
mammoth-pharmacist-54215
08/21/2015, 5:09 PM@ancient-stone-50795: I believe I addressed all your concerns in your review. - a
ancient-stone-50795
08/21/2015, 5:10 PM@mammoth-pharmacist-54215: sorry, I already gave you a ship it so I didn’t chime in further. I’ll take another look. - m
mammoth-pharmacist-54215
08/21/2015, 5:11 PMNo worries, my last update actually fixed some issues with UTF-8 handling I noticed yesterday. - a
ancient-stone-50795
08/21/2015, 5:11 PMI know this is mostly just a port from twitter commons. Was it in a working state before? - e
enough-analyst-54434
08/21/2015, 5:11 PMYes - both twitter and aurora use the checkstyle tool - m
mammoth-pharmacist-54215
08/21/2015, 5:12 PMthere was a problem parsing files that contained the utf-8 coding line - e
enough-analyst-54434
08/21/2015, 5:12 PMI've been a bad reviewer, but in retrospect landing the code import 1st, then using it as a library in a 2nd RB may have been alot smoother - m
mammoth-pharmacist-54215
08/21/2015, 5:12 PMseems to be a bug in ast.parse - m
mammoth-pharmacist-54215
08/21/2015, 5:13 PMI added a comment in the code that explains why I’m doing the odd replacement of that line. - a
ancient-stone-50795
08/21/2015, 5:13 PM@mammoth-pharmacist-54215: looks like it passed CI 3 days ago. Did your last commit have a travis-ci test run? - m
mammoth-pharmacist-54215
08/21/2015, 5:14 PMyes for some reason last time I pushed it I got a new pull request… I updated the rb with the link - a
ancient-stone-50795
08/21/2015, 5:14 PMIt is difficult to review something so big. Its got a bunch of complex code in it but I kind of assume that is already working. - m
mammoth-pharmacist-54215
08/21/2015, 5:14 PM - m
mammoth-pharmacist-54215
08/21/2015, 5:15 PMmost of the code that actually checks things is unmodified and should work as well as it currently does in the native checkstyle. - g
gorgeous-fish-11040
08/21/2015, 5:15 PM@enough-analyst-54434 @bored-art-40741 while trying to make my change I found a few bugs in handling Python interpreters. Here is an RB: https://rbcommons.com/s/twitter/r/2679/ - m
mammoth-pharmacist-54215
08/21/2015, 5:15 PMI’m mostly concerned about making sure its plumbed into pants sanely - a
ancient-stone-50795
08/21/2015, 5:17 PM@mammoth-pharmacist-54215: seems OK to me. Are you looking for someone to commit it? - m
mammoth-pharmacist-54215
08/21/2015, 5:19 PM@ancient-stone-50795: thanks - m
mammoth-pharmacist-54215
08/21/2015, 5:19 PMyeah, I don’t have access to commit I believe. - a
ancient-stone-50795
08/21/2015, 5:21 PM@witty-crayon-22786: and @enough-analyst-54434 participated in the review. If they don’t have any further objections I’ll patch it in later today (after the release - I wouldn’t want to get in the way of that) - e
enough-analyst-54434
08/21/2015, 5:22 PMI have no objections to a patch in before the release - still a few hours away if a revert is needed for whatever reason - m
mammoth-pharmacist-54215
08/21/2015, 5:22 PMStu is in out of town until Monday - e
enough-analyst-54434
08/21/2015, 5:22 PMThis is an opt-in task, so its pretty release safe - m
mammoth-pharmacist-54215
08/21/2015, 5:23 PMyup, people may have to update their pants.ini file to disable it if they use something other than the pants.ini we provide