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

Refactor Feature/Module/connector => "Plugin" (Third try) #593

Closed
wants to merge 6 commits into from

Conversation

aneilbaboo
Copy link

See discussion at #565

Renamed modules folders to plugins
Renamed connector.js to plugin.js
Renamed Feature class to Plugin
Renamed modules variables to plugins
Renamed misc other Module___.___ files to Plugin___.___

@Vlasenko - This is the third try. I was careful to do directory renamings first, followed by file renamings, then alterations of file contents, each as separate commits, but git/Github is treating a couple of graphql files as different. It's weird because neither the filename nor contents changed, just the directory. I've spent a few hours on this taking different approaches, but it's not working.

Can we get this merged?

Aneil Mallavarapu added 6 commits December 23, 2017 12:09
First step of Plugin refactor. Preserving file history,
however, tests will fail for this commit.
Including files in /src/client, src/server and tools
Improves clarity and eases the next step of the plugin refactor
Renames all references to the Feature class
* Rename variables and class names to plugin[s], and Plugin
* Correct references to files renamed in previous steps
@aneilbaboo aneilbaboo changed the title Refactor Feature/Module/connector => "Plugin" (Second try) Refactor Feature/Module/connector => "Plugin" (Third try) Dec 23, 2017
@larixer
Copy link
Member

larixer commented Dec 24, 2017

@aneilbaboo It cannot be merged, because it is not only couple graphql files, I see many files in the tools dir where Git has lost track of renamings

@larixer
Copy link
Member

larixer commented Dec 24, 2017

In your last commit: 66d236f
you are doing a bunch of renamings and change the contents of the files at the same time. Of course Git losts track of files because of that

@larixer
Copy link
Member

larixer commented Dec 24, 2017

@aneilbaboo I'm looking to have two commits in this pull request. The first commit does all the renamings. The second commit changes all the files contents as needed. Please be careful with that and use git rebase to group your commits. You are trying to do non-trivial thing and it should be done carefully

@aneilbaboo
Copy link
Author

@Vlasenko - I see what's been happening now. I created a branch, on my repo, then merged that branch into my master on Github. Github doesn't do a fast forward merge. It squashes all the commits together. Let me fix it.

screen shot 2017-12-24 at 1 00 01 am

@larixer
Copy link
Member

larixer commented Dec 24, 2017

@aneilbaboo Lets hold off with that. I think I should document current approach carefully first, then we will think about better naming. I plan to do so on the second week of January, as I'm on a vacation right now

@larixer
Copy link
Member

larixer commented Dec 24, 2017

@aneilbaboo Will plugin still be a good name if/when we start npm-like repo for them?

@larixer
Copy link
Member

larixer commented Dec 24, 2017

Maybe plugins is a good name. Imagine you have npm-like repo and you install these plugins into your project. To install them you need to have right socket in your project that these plugins can fit into. So having client/plugins/plugin.js is misleading in this sense, its better to have client/plugins/socket.js right?

@aneilbaboo
Copy link
Author

aneilbaboo commented Dec 24, 2017

@Vlasenko - I'll hold off on anymore changes.

I think plugin works for an npm-like service. Grunt has plugins, for example. I'm guessing they use that word to avoid collision with npm's modules. Chrome and Firefox used to have plugins and a site for accessing them. I think they both switched to different formats ("extensions" and "add-ons") in recent versions.

@larixer
Copy link
Member

larixer commented Dec 24, 2017

@aneilbaboo Yes, I think plugins should work for us too. But naming their interface as plugin in src/plugins/plugin.js, might be not very good idea, rather it should be connector as it is now, or socket or something like that...

@larixer larixer force-pushed the master branch 29 times, most recently from 22cc234 to 06fd4b4 Compare January 22, 2018 15:20
@aneilbaboo aneilbaboo closed this Apr 17, 2018
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.

2 participants