opengrok
opengrok copied to clipboard
use hashmap for groups too
projects are using hashmap (#1543) for fast lookup, the same should be implemented for groups
I would like to work on this. Please let me know if required. Thanks!
Feel free to take it !
It's a free food!
I would like to work on this.
@vladak @tulinkry I am new to this so can you guide me on where should I start? I know basic java.
The goal is to convert groups in Configuration to use HashMap. See https://github.com/oracle/opengrok/blob/5f88acb4406c8e728ddf626ec6d37eb8b5d81e34/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java#L151-L152
Thanks, @vladak, I would like to contribute too.
so basically I have to convert the Set into a hash map? @vladak
so basically I have to convert the Set into a hash map? @vladak
Yep !
@vladak I should use Map<String, String> or Map<String, Group>? Please help
I am actually referring to this source: https://stackoverflow.com/questions/16108734/convert-setmap-entryk-v-to-hashmapk-v#:~:text=6%20Answers&text=There%20is%20no%20inbuilt%20API,using%20Entry%20fill%20in%20map.&text=Returns%20a%20Set%20view%20of,set%2C%20and%20vice%2Dversa. Hope I am doing it right? @vladak
@vladak I should use Map<String, String> or Map<String, Group>? Please help
the latter. It should mimic what is done for projects.
@vladak sir, I am sharing the code snippet can you please look into it and guide me if I have done it wrong? //Converting set into map private Map<String ,Group> group_map = new HashMap<>(); // filling the map private Set<Entry<String,Group>> groups = group_map.entrySet(); private Map<String,Group> new_group = new HashMap<>(); // iterating through the set for(Entry<String,Group> entry : groups) { mapFromSet.put(entry.getKey(),entry.getValue()); }
No, you need to convert the code so that it uses the HashMap natively, not just a conversion from the existing set.
so @vladak sir you mean that I have to convert only these two lines?
private Map<String, Project> projects; // project name -> Project
private Set<Group> groups;
and it uses Hash map natively?
Sorry sir I know I have been asking a lot of questions since it's my first time contributing to a proper code kindly acknowledge.
Yep, begin with:
private Map<String, Project> projects; // project name -> Project
private Map<String, Group> groups; // group name -> Group
and adapt the rest of the code.
oh ok, I thought this only initially but then referred to the net. Thanks a lot sir@vladak
@vladak sir can you please now review my Pull Request?
@vladak sir is it fine sir or any changes are required ?
You need to see how exactly is the group member used and adapt the rest of the code. This is definitely not a one liner change. Read the code extensively first and become familiar with how the project groups work. Then proceed to make any changes.
Also, run the tests before submitting PRs.
ok sir
Hello @vladak @tulinkry , this is my first time contributing to an Open Source project, and am still learning the process. I have tried to make the desired changes as told in the issue and was wondering how I can test if my changes are working. I would really appreciate your mentoring and any help that you can provide. Thanks!
Edit: I have never worked on a production level project. Hoping to make a useful contribution while learning new things and improving myself.
Seems like we will have races of who will fix this first :-) Feel free to submit a PR. As for the testing, you can totally do it yourself. Just create a setup with project groups, see https://github.com/oracle/opengrok/pull/3466
Yep, begin with:
private Map<String, Project> projects; // project name -> Project private Map<String, Group> groups; // group name -> Groupand adapt the rest of the code.
do I just need to make changes in Configuration.java so that the above changes work or should I make changes to methods like setGroup(Set<Group> groups) -> setGroup(Map<String, Group> groups). I am confused as to what extent I should make changes!
Definitely the setters/getters need to adapt. And any other code that uses the value returned from the getter. Just keep bubbling/propagating this.
Hi @adityamathur97 are you still working on this ? or can I take it ?
As issue is still open I will pick this up.
Is this issue closed?
The status says open and there is PR #3873 in progress. Looks like there is a bit of contention for this one :-) There are lots of other, potentially more interesting issues to work on.
Hi, Is it still open? Can I contribute to it? I have gone through the configuration.java file and found that changes will be required on multiple place on the file. Can I start working on it?