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

Added CefSharp.Core #280

Closed
wants to merge 14 commits into from
Closed

Added CefSharp.Core #280

wants to merge 14 commits into from

Conversation

JanEggers
Copy link
Contributor

I stripped off all the changes related to tasks and use of (M)CefRefPtr instead of *. So guess this is a clean basic version of my former changes that will follow later.

I also did a Whitespace cleanup run with no space after keywords this produces some whitespace changes elsewhere so i dont know what is the intended setting please tell me and i will fix it.

if there are any changes required just tell me.

@perlun
Copy link
Member

perlun commented Mar 20, 2014

Thanks, Jan! Regarding the whitespace, it should be if (state == STATE_DEFAULT), i.e. with whitespace after keywords.

I am doing some minor tweaks to get stuff compiling w/ VS2012 on this one, and will push it up as a separate feature branch in the main repo that we can then work on finalizing together.

@perlun
Copy link
Member

perlun commented Mar 20, 2014

I pushed up a feature branch now, here: https://github.com/cefsharp/CefSharp/tree/with-cefsharp-as-c-sharp-instead-of-cpp

We can use it as a WIP while we work with the code. I haven't had time to review it that much yet; I managed to get the WPF.Example compiling w/ VS2012 (after applying a bit of duct tape 😄) and it seems to basically work - good work, Jan!

Next step is perhaps to clean up the C# bits a bit and go through things and do a proper code review. Jan, I suggest you don't make too much changes of this until I've had time to do that. Thanks for all your work so far. Feel free to work in the meanwhile by cleaning up the other stuff we have pending for 3.29.0 (I saw you already did one of them, which was really great).

@perlun
Copy link
Member

perlun commented Mar 21, 2014

So there. I've merged most of it now. There was one commit I added (namely 9f1ae90) that we should discuss. I didn't fully see the point with the ObjectBase base class. We should probably give it a better name, and possibly even write a bit of a comment on it, explaining its raison-d'être.

All in all, very good changes Jan! We should also start moving to have as much of the public C# API documented now, using XML /// notation. It's all very easy now, thanks to the fact that much of the code people will actually have to care about is now C# and not some awkward C++/CLR junk... 😃

@perlun perlun closed this Mar 21, 2014
@JanEggers JanEggers deleted the CefSharp.Core branch March 22, 2014 08:03
@jornh jornh added this to the 3.29.0 milestone Mar 24, 2014
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.

None yet

3 participants