pantsbuild #development

Channels

welcome

github-notifications

development

witty-crayon-22786

04/27/2016, 12:28 AM

@wide-energy-11069: yea, that's a good start. might be worthwhile to break that ticket into two reviews... one to replace the boilerplate collections

witty-crayon-22786

04/27/2016, 12:29 AM

and then another to allow for some native merging

witty-crayon-22786

04/27/2016, 12:30 AM

@wide-energy-11069: actually, one bit

wide-energy-11069

04/27/2016, 12:30 AM

which part needs to be native exactly?

full-kilobyte-93429

04/27/2016, 12:30 AM

now i made the

ScmProjectTree

pickable, some tests are failing with git errors, looks like there is some initialization not done, just to check that's expected @witty-crayon-22786

witty-crayon-22786

04/27/2016, 12:30 AM

there is no need to declare the singleton

Files

thing, per say... you could do:

witty-crayon-22786

04/27/2016, 12:30 AM

Select(Collection.of(File))

witty-crayon-22786

04/27/2016, 12:30 AM

@full-kilobyte-93429: yea, totally expected

full-kilobyte-93429

04/27/2016, 12:31 AM

cool!

witty-crayon-22786

04/27/2016, 12:31 AM

@full-kilobyte-93429: if you're able to fix the initialization, that would be great... anything else, can mark known to fail

witty-crayon-22786

04/27/2016, 12:31 AM

@wide-energy-11069: the native collection merging would be not needing to declare a task to merge single `T`s into a

Collection.of(T)

of something

full-kilobyte-93429

04/27/2016, 12:32 AM

sure, biggest thing is some exception classes are inner and need to move to the top level, these are marked as public api

full-kilobyte-93429

04/27/2016, 12:32 AM

so need add some deprecation logic

witty-crayon-22786

04/27/2016, 12:33 AM

@full-kilobyte-93429: hm... aren't those on the reader?

witty-crayon-22786

04/27/2016, 12:33 AM

@full-kilobyte-93429: basically, making at least 1 test_fs.py git test work

witty-crayon-22786

04/27/2016, 12:34 AM

even one would be fine.

full-kilobyte-93429

04/27/2016, 12:37 AM

yea, some exception classes are from

GitRepositoryReader

some from

Scm

full-kilobyte-93429

04/27/2016, 12:40 AM

have two tests passed

full-kilobyte-93429

04/27/2016, 12:40 AM

the rest all fail for the same error

witty-crayon-22786

04/27/2016, 4:07 AM

@full-kilobyte-93429: what i mean is: there shouldn't be any need to pickle the reader

witty-crayon-22786

04/27/2016, 4:07 AM

in JVM parlance, it would be a "transient" field

witty-crayon-22786

04/27/2016, 4:07 AM

alternatively, if you make it a memoized field, it should just work, because the memo is stored on the method, rather than on the class.

full-kilobyte-93429

04/27/2016, 4:57 PM

not pickling the reader but the code threw exceptions that are inner classes which aren't pickable.

witty-crayon-22786

04/27/2016, 6:16 PM

@full-kilobyte-93429: so, again: i'm pretty sure this should not be necessary

witty-crayon-22786

04/27/2016, 6:16 PM

before you change too much more, can you post a preliminary review?

full-kilobyte-93429

04/27/2016, 6:17 PM

sure, i can just publish what i have now

full-kilobyte-93429

04/27/2016, 6:26 PM

https://rbcommons.com/s/twitter/r/3770/ @witty-crayon-22786 worth to mention without diff2 it's pickable in non-fast mode, but fail in

fast

mode

witty-crayon-22786

04/27/2016, 6:27 PM

@full-kilobyte-93429: if the reader were a

memoized_property

of scm_project_tree, it would not need to be pickleable at all

witty-crayon-22786

04/27/2016, 6:27 PM

and only the scm would need to be pickleable

full-kilobyte-93429

04/27/2016, 6:28 PM

sure, i will try that, suspect the exceptions still need to be pickled