Fork me on GitHub
#clojure-dev
<
2017-02-17
>
Alex Miller (Clojure team)04:02:23

thx - is that a patch over current state or clean patch?

hiredman08:02:45

it applies cleanly to master

Alex Miller (Clojure team)15:02:13

@hiredman tests were failing with patch due to typo. I fixed, but found something else that looks amiss - details in patch

hiredman16:02:47

I fiddled with the tag metadata in the patch too. I had been tagging exceptions with java.lang.Throwable, but it seems more correct to pull the tag from the analyzer analysis, but that tag is a class and not a symbol.

hiredman16:02:42

and the 0004 patch I just uploaded (sorry, I didn't notice your -4 patch until now) doesn't apply cleanly to master because I didn't pull master before I created it, so here comes 0005

PB16:02:24

Hey all. I’d like to start trying to contribute to clojure core. I have signed the agreement and I’m looking through the jira. I was looking at starting with the trivial bugs first as I’m rather unfamiliar with the codebase but all seem to have already been patched. Does this work on a first come basis? What would be the most productive use of my/your time when it comes to this?

bronsa16:02:05

@petr it's usually a process of: - find an open bug you're interested in fixing and that's within your capabilities - fix it - wait a bit

bronsa16:02:24

there's tons of open tickets you can look at

bronsa16:02:42

some are marked as trivial, you could start there

PB16:02:57

@bronsa is it safe to assume that any of the issues reported here have been approved as ready to work?

bronsa16:02:36

@petr all the issues marked as vetted in JIRA that have no patch are tickets that have been approved as ready to work

bronsa16:02:47

here's a diagram of how clojure's jira is managed

bronsa17:02:23

@hiredman can you type hint that call to .getName

bronsa17:02:13

also calling symbol on it shouldn't be necessary as string type hints are fine

hiredman17:02:55

ah, right, actually, come to thing of it, would tag ever not contain a class coming from the analyzer?

bronsa17:02:11

the analyzer always assocs a tag as a Class

bronsa17:02:21

because I couldn't bother dealing with either Class/symbol or strings

hiredman17:02:40

sure, I was thinking more is tag always present

bronsa17:02:24

do something like (vary-meta x merge (when tag {:tag tag}))

bronsa17:02:40

i actually missed doing that in my own patch, will need to follow it up

bronsa17:02:54

not a big deal tbh, the compiler will ignore {:tag nil} anyway

bronsa17:02:04

but it's less code to emit

bronsa17:02:23

go is emitting quite a lot of dead code ATM

bronsa17:02:40

i want to try and reduce it

hiredman17:02:43

the cljs version and the clojure version are also entirely different

bronsa17:02:33

i wish i cared about cljs as much as i care about clj but I dont ¯\(ツ)

bronsa18:02:07

also i have no idea if any of those memory issues even affect js engines

hiredman18:02:57

in theory a compiler (pure function, data in, data out) is the sort of thing you should be able to share easily between clj and cljs

bronsa18:02:40

cljs and clj have semantics different enough to make that difficult

bronsa18:02:12

+ different vms = different memory/perf concerns

bronsa18:02:35

and the lack of t.a.js for the cljs port makes it extra difficult to port stuff over

bronsa18:02:54

@alexmiller i have a couple of one-line commit to add on top of last patch, want me to make another ticket or attach them to the same?

Alex Miller (Clojure team)18:02:39

bleh. I hate everything about applying multiple interleaving patches to multiple tickets.

Alex Miller (Clojure team)18:02:48

I’m on the verge of unrolling all of them

Alex Miller (Clojure team)18:02:57

and starting over with a single patch for each

Alex Miller (Clojure team)18:02:33

the thread thing seems like an independent ticket

Alex Miller (Clojure team)18:02:46

so I’d make that separate

bronsa18:02:48

the other one is not a regression but just a cleanup

Alex Miller (Clojure team)18:02:57

yeah, I read the back chat

bronsa18:02:26

just let me know what's easiest for you

Alex Miller (Clojure team)18:02:37

I think I am going to unroll these changes so we can get a single commit that represents the patch

bronsa18:02:24

k, I'll make a separate ticket wrt thread leaking then

bronsa18:02:01

it's been a couple of really painful days, sorry about that :)

hiredman18:02:11

let me know if you want me to do anything (re: squashing, generating a single patch, etc)

Alex Miller (Clojure team)18:02:13

I’m not expecting to get to the point of a release on this till early next week at this point as it needs to get run through internal testing on stuff I don’t even have access to

Alex Miller (Clojure team)18:02:25

@hiredman yeah, I’d like a single squashed patch for 138

Alex Miller (Clojure team)18:02:40

but let me back out the partial changes first to give you something to build against

hiredman18:02:46

maybe you mean 169? (or I guess both, but 169 is my fault)

Alex Miller (Clojure team)18:02:37

repo should be back to unapplied state for 169 and 138

Alex Miller (Clojure team)18:02:05

@petr for a variety of reasons, there are not a lot of good newbie places to start in the Clojure jira. Most tickets tend to fall into one of: 1) good problem, easy to fix - those typically have already have a patch and are awaiting eval 2) good problem, clear idea to fix, but hard to implement 3) good problem, but no clear idea on path to a fix 4) problem/enhancement that has not been evaluated and may not be wanted/needed

Alex Miller (Clojure team)18:02:21

most people get started working on something because it is a problem that is actually affecting them

bronsa18:02:19

aight, reopened and attached -v6 to ASYNC-138, opened ASYNC-185 for the thread leak

bronsa18:02:20

i can now put in my CV driven alex miller insane by forcing him to revert my patches 3 times :)

bronsa18:02:49

i'll try harder next time

hiredman18:02:55

you know, apparently the linux kernel has merge commits with 66 parents

bronsa18:02:16

I didn't even know that was a thing that was possible

bronsa18:02:50

haha >>>It's pulled, and it's fine, but there's clearly a balance between "octopus merges are fine" and "Christ, that's not an octopus, that's a Cthulhu merge".

PB18:02:59

@alexmiller that’s unfortunately what I’m finding. I have no problem working on more difficult problems, it’s just that I don’t know the clojure core to the point that I would feel all that comfortable in doing so. As per @bronsa ‘s recommendation I have found this list of issues: http://dev.clojure.org/jira/secure/IssueNavigator.jspa?mode=hide&amp;requestId=10373. It is a whole lot smaller than I had imagined but it does give me an idea of what I could work on without being too worried that the ticket would be rejected as a whole.

Alex Miller (Clojure team)18:02:44

well, I would say that many of those tickets are in that bucket but have not actually been approved by Rich. I’ve stashed them there just so they get looked at

PB18:02:21

In that case I’m unfortunately at a loss of where to start

Alex Miller (Clojure team)18:02:06

unfortunately I don’t have anything specific to point to at the moment

Alex Miller (Clojure team)18:02:26

things are pretty far backed up waiting for Rich atm

PB18:02:32

Sorry, I didn’t mean to imply that I’d like you to spoon feed me tickets. I just don’t want to waste my time writing a patch for something that is not wanted and as a result wasting the time of other people

bronsa18:02:53

@alexmiller FYI apparently applying 138->169->185 causes conflicts but 169->138->185 applies fine

Alex Miller (Clojure team)18:02:28

I’m probably not going to do this today as I am going to turn off the computer and go outside :)

bronsa18:02:52

that's always a good idea