Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimizing java code #65

Closed
wants to merge 2 commits into from
Closed

Optimizing java code #65

wants to merge 2 commits into from

Conversation

bloderxd
Copy link

What's this?

First of all... Good job about clojure!

I'd like to make clojure code better, then I did some optimization in Fn interface and abstract class, if you approve it, I can also optimize other classes.

@jafingerhut
Copy link
Contributor

The Clojure project does not accept pull requests. If you want to suggest proposed improvements, you can file a JIRA ticket here: http://dev.clojure.org/jira/browse/CLJ

Also, I briefly scanned through your proposed changes, and it looks like reformatting some code with only changes in line breaks and/or white space? If so, I'll simply mention that such changes are typically low priority compared to other changes that fix bugs, improve run time performance, or add new features.

@bloderxd
Copy link
Author

Yes, it's not high priority but I saw java code and I thought very ugly, then I just would like to make it better to other people who will see it.

@bloderxd
Copy link
Author

and optimize this code doesn't involve just break lines and white spaces but also some if, switch logics

@jafingerhut
Copy link
Contributor

@bloder You can open whatever tickets on JIRA for whatever changes you are interested in. I'm simply trying to tell you that prettyifying ugly code has been desired by many people besides you, and so far such changes have been considered very low priority. I don't make the decisions of what changes are made to Clojure and which are not -- just passing on some info about the past.

@puredanger
Copy link
Member

@bloder we're not interested in these changes, sorry - please close the PR.

@bloderxd bloderxd closed this Apr 18, 2016
@AeroNotix
Copy link

Where are the optimizations?

@bloderxd
Copy link
Author

This is just an example to suggest the code optimization.

@zachncst
Copy link

This has an interesting side affect of making @bloder look like he's contributed to Clojure. Very sketchy.

@bloderxd
Copy link
Author

I just did these simple modifications because I saw they don't accept pull requests but I'd like to optimize other classes too,

look this one - https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/EdnReader.java

This PR is just to example what I'd like to do about clojure java code, it's better do this example and wait them answers than optimize all java code and they close my PR.

@neverfox
Copy link

You keep saying "optimize", but I don't think it means what you think it means.

@thiagofm
Copy link

@bloder it seems they are too busy to fix those small code styling issues, as there are none "optimizations" being done, they would rather focus on other issues. Also, they don't accept contributions from here. I advise you look somewhere else to start contributing.

@neverfox @zachncst I really see no need for this kind of comments, he's just trying to help. I actually think that his contributions makes sense and probably took some digging, perhaps he was reading the source and found it could be better. I maintain software and I don't mind merging typos in the readme. I think every contribution is equal, given that it makes sense for the project. Please be kind.

@AeroNotix
Copy link

@bloder fixing whitespace issues is not optimization. Do you have an example of where you would optimize the Java code? I believe if you had some real optimizations (again, whitespace changes are not optimizations) then you could get a bit more traction with your PR (albeit on Jira, not GitHub).

@bloderxd
Copy link
Author

@AeroNotix Ok, I'll try to refactor some big if or for or switch and make other commit here, ok? Sorry if I'm offending some one.

@AeroNotix
Copy link

AeroNotix commented Apr 19, 2016

No, clearly the maintainers don't want you to do that. Again, the difference between whitespace changes and optimizations is huge. Do not confuse the two.

If you have some actual optimizations then head over to JIRA and explain which pieces of code need optimizing, optimize them, measure the optimization and compare to the unoptimized code.

Whitespace changes are not optimizations.

@bloderxd
Copy link
Author

Yes I know that, my mistake in this PR was just remove whitespaces to example my idea, sorry about it, a good example that I saw was two or more for's doing the same thing in different functions changing only what ISec field will receive, my idea is optimize it creating a new solution to don't repeat it (DRY), like a function, I know my example was ugly but I hope you understand my idea, it's not about whitespaces, it's about code design. And don't worry I understood you don't accept contribution by github.

@neverfox
Copy link

@bloder I apologize for misinterpreting your intentions. I've come across a kind of PR troll before that is primarily trying to turn the PR into some sort of commentary on or outright ridicule of the project. In my haste, I took this to be something along those lines and I was wrong about that. In any case, good luck with your proposals over on JIRA. Cheers. @thiagofm

@thiagofm
Copy link

@neverfox no problem. I also struggle with them. I guess it's always a good idea to think of others having good intentions upfront, I know it's hard, but I guess by doing that we can have a better community.

@bloderxd
Copy link
Author

@neverfox no problem, it's also my fault that could not express myself properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants