Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Fixing build issues on Ubuntu 18.04 #645

Closed
wants to merge 0 commits into from
Closed

Fixing build issues on Ubuntu 18.04 #645

wants to merge 0 commits into from

Conversation

g-217
Copy link
Contributor

@g-217 g-217 commented May 4, 2018

No description provided.

@@ -1,6 +1,7 @@
#include "appshell/appshell_extensions_platform.h"
#include <unicode/ucsdet.h>
#include <unicode/ucnv.h>
#include <unicode/unistr.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the headers be added only for LINUX. Why add unnecessary headers on WIN/MAC if not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll move these header inclusion to #ifdef OS_LINUX

@@ -34,6 +34,9 @@
#ifdef OS_LINUX
#include <gtk/gtk.h>
#endif
#include <unicode/ucnv_err.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above for Linux only headers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll move these header inclusion to #ifdef OS_LINUX

@@ -337,13 +337,17 @@ bool ClientHandler::OnDragEnter(CefRefPtr<CefBrowser> browser,
}
#endif

/**
* We do not plan to add any feature to parent class(::ClientHandler) implementation of this function,
* So override is useless
void ClientHandler::OnDraggableRegionsChanged(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the override causing a warning or an error? Do we need to change this on all platforms?

Test these changes on all platforms and make sure nothing breaks on WIN/MAC due to this.


/**
* We do not plan to add any feature to parent class(::ClientHandler) implementation of this function,
* So override is useless
void OnDraggableRegionsChanged(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above on override

namespace{
template<typename T>
struct deleter
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these changes if not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving these changes to a branch rather than the master branch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants