search

gephi / gephi-plugins

Repository for Gephi Plugins maintained by the team. Each plugin has it's branch.
270 stars 623 forks source link

cloned from eduardo's branch, added commits #151

Closed seinecle closed 7 years ago

seinecle commented 7 years ago

hope this is ok...

seinecle commented 7 years ago

I might have forgotten to increment the version number

eduramiba commented 7 years ago

You can just push a new commit to update this pull request without opening a new one.

This still reverts my changes anyway. What is the fix you did? I guess it should be one or two files changed only but this changes 11 files. Can't you redo the changes based on my commit instead of overwriting all files with the old commit files that you worked on?

seinecle commented 7 years ago

I did many changes based on user comments: important perf increases and handling of null values. Plus better comments / removal of unnecessary variables. This impacts many files. So I tried to start from your files but in some cases I just pasted new files entirely. I'll do a commit with new version number

seinecle commented 7 years ago

version number incremented

eduramiba commented 7 years ago

I think I have now included all your new changes without undoing my previous changes.

Please check my branch here: https://github.com/eduramiba/gephi-plugins/tree/similaritycomputer (clone, build and run it. Then test that it works completely as it should now. If it does, then you should force push that to your branch and do the pull request this way).

eduramiba commented 7 years ago

If you are wondering, many of the changes are about not using static variables (specially this) and class fields where it's not necessary.

That way, it's hard do follow, predict, test and debug what the code will do when a method is called several times (for example the plugin is used more than once). In fact, it would be nice to get rid of the 2 class fields left in VectorsBuilder but I didn't because I wasn't sure I would not break anything if I made them method local variables, which makes my previous point more evident.

seinecle commented 7 years ago

I was wondering actually :-) thx a lot for explaining, makes it easier to understand how important this is - I thought this was just about style (which can be important, too...) I'll work on it!