Merge into trunk : expand-filter-support : Code : unity-js-scopes

Reviewer	Review Type	Date Requested	Status
Marcus Tomlinson (community)		2016-05-30	Needs Fixing on 2016-09-06
unity-api-1-bot	continuous-integration		Approve on 2016-08-17
PS Jenkins bot	continuous-integration		Pending
Review via email: mp+296067@code.staging.launchpad.net

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-05-30:

#

FAILED: Continuous integration, rev:143
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/9/
Executed test runs:
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/74/console
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/81/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/9/rebuild

review: Needs Fixing (continuous-integration)

Reply

lp://staging/~abreu-alexandre/unity-js-scopes/expand-filter-support updated on 2016-05-31

144. By Alexandre Abreu on 2016-05-31: Ooops forgot part of the commits, object constructors

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-05-31:

#

FAILED: Continuous integration, rev:144
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/10/
Executed test runs:
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/76/console
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/83/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/10/rebuild

review: Needs Fixing (continuous-integration)

Reply

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-06-06:

#

Yeah, looks mostly done. A few gaps here and there though.

You've only bound one constructor for the RangeInputFilter, ValueSliderFilter and ValueSliderLabels classes. We need to do the "v8::FunctionCallbackInfo<v8::Value> const& args" thing for each of these in order to cover the available overrides.

Also, could you please test against the example scope I wrote in lp:~marcustomlinson/unity-js-scopes/filters_example. I suspect there will be other errors, but already I'm seeing:

/home/marcustomlinson/Projects/work/unity-js-scopes/filters_example/examples/filters-build/src/filters.marcustomlinson_filters.js:471
var price_filter = new scopes.lib.RangeInputFilter("price", "Min", "", "to", "Max", "", group);
^

Error: argument count does not match the corresponding C++ function definition
at Error (native)
at null.<anonymous> (/home/marcustomlinson/Projects/work/unity-js-scopes/filters_example/examples/filters-build/src/filters.marcustomlinson_filters.js:471:38)

review: Needs Fixing

Reply

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2016-06-06:

#

> Yeah, looks mostly done. A few gaps here and there though.
>
> You've only bound one constructor for the RangeInputFilter, ValueSliderFilter
> and ValueSliderLabels classes. We need to do the
> "v8::FunctionCallbackInfo<v8::Value> const& args" thing for each of these in
> order to cover the available overrides.
>
> Also, could you please test against the example scope I wrote in
> lp:~marcustomlinson/unity-js-scopes/filters_example. I suspect there will be
> other errors, but already I'm seeing:
>
> /home/marcustomlinson/Projects/work/unity-js-scopes/filters_example/examples
> /filters-build/src/filters.marcustomlinson_filters.js:471
> var price_filter = new scopes.lib.RangeInputFilter("price",
> "Min", "", "to", "Max", "", group);
> ^
>
> Error: argument count does not match the corresponding C++ function definition
> at Error (native)
> at null.<anonymous> (/home/marcustomlinson/Projects/work/unity-js-
> scopes/filters_example/examples/filters-
> build/src/filters.marcustomlinson_filters.js:471:38)

yes I remember chose to bind the most "complete" constructor ...
I used your example with null & null for the extra values,

Agree that it should be updated with varargs support though,

Reply

lp://staging/~abreu-alexandre/unity-js-scopes/expand-filter-support updated on 2016-06-06

145. By Alexandre Abreu on 2016-06-06: Add varargs support for ValueSliderFilter & RangeInputFilter constructors

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2016-06-06:

#

updated

Reply

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-06-06:

#

FAILED: Continuous integration, rev:145
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/11/
Executed test runs:
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/77/console
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/84/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/11/rebuild

review: Needs Fixing (continuous-integration)

Reply

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-06-07:

#

Download full text (4.5 KiB)

This is what I get now, running the filters example scope:

#
# Fatal error in ../deps/v8/src/api.h, line 412
# Check failed: that == __null || (*reinterpret_cast<v8::internal::Object* const*>(that))->IsJSObject().
#

==== C stack trace ===============================

1: V8_Fatal
2: v8::Utils::OpenHandle(v8::Object const*, bool)
3: v8::Object::Has(v8::Local<v8::Value>)
4: ValueSliderLabels::ValueSliderLabels(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, v8::Local<v8::Value>)
5: ValueSliderLabels* v8cpp::internal::ObjectFactory<ValueSliderLabels>::new_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value> >(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)
6: v8cpp::internal::FunctionTraits<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)>::ReturnType v8cpp::internal::call_from_v8_impl<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), 0ul, 1ul, 2ul>(ValueSliderLabels* (*&&)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), v8::FunctionCallbackInfo<v8::Value> const&, v8cpp::internal::CallFromV8Traits<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), 1ul>, v8cpp::internal::IntSequence<unsigned long, 0ul, 1ul, 2ul>)
7: v8cpp::internal::FunctionTraits<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)>::ReturnType v8cpp::internal::call_from_v8<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)>(ValueSliderLabels* (*&&)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), v8::FunctionCallbackInfo<v8::Value> const&)
8: void v8cpp::internal::Class<ValueSliderLabels>::set_constructor<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_tra...

This is what I get now, running the filters example scope:

#
# Fatal error in ../deps/v8/src/api.h, line 412
# Check failed: that == __null || (*reinterpret_cast<v8::internal::Object* const*>(that))->IsJSObject().
#

==== C stack trace ===============================

1: V8_Fatal
 2: v8::Utils::OpenHandle(v8::Object const*, bool)
 3: v8::Object::Has(v8::Local<v8::Value>)
 4: ValueSliderLabels::ValueSliderLabels(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, v8::Local<v8::Value>)
 5: ValueSliderLabels* v8cpp::internal::ObjectFactory<ValueSliderLabels>::new_object<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value> >(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)
 6: v8cpp::internal::FunctionTraits<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)>::ReturnType v8cpp::internal::call_from_v8_impl<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), 0ul, 1ul, 2ul>(ValueSliderLabels* (*&&)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), v8::FunctionCallbackInfo<v8::Value> const&, v8cpp::internal::CallFromV8Traits<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), 1ul>, v8cpp::internal::IntSequence<unsigned long, 0ul, 1ul, 2ul>)
 7: v8cpp::internal::FunctionTraits<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)>::ReturnType v8cpp::internal::call_from_v8<ValueSliderLabels* (*)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>)>(ValueSliderLabels* (*&&)(v8::Isolate*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value>), v8::FunctionCallbackInfo<v8::Value> const&)
 8: void v8cpp::internal::Class<ValueSliderLabels>::set_constructor<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value> >()::{lambda(v8::FunctionCallbackInfo<v8::Value> const&)#1}::operator()(v8::FunctionCallbackInfo<v8::Value> const&) const
 9: std::_Function_handler<ValueSliderLabels* (v8::FunctionCallbackInfo<v8::Value> const&), void v8cpp::internal::Class<ValueSliderLabels>::set_constructor<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, v8::Local<v8::Value> >()::{lambda(v8::FunctionCallbackInfo<v8::Value> const&)#1}>::_M_invoke(std::_Any_data const&, v8::FunctionCallbackInfo<v8::Value> const&)
10: std::function<ValueSliderLabels* (v8::FunctionCallbackInfo<v8::Value> const&)>::operator()(v8::FunctionCallbackInfo<v8::Value> const&) const
11: v8cpp::internal::Class<ValueSliderLabels>::export_object(v8::FunctionCallbackInfo<v8::Value> const&)
12: v8cpp::internal::Class<ValueSliderLabels>::Class(v8::Isolate*, unsigned long)::{lambda(v8::FunctionCallbackInfo<v8::Value> const&)#1}::operator()(v8::FunctionCallbackInfo<v8::Value> const&) const
13: v8cpp::internal::Class<ValueSliderLabels>::Class(v8::Isolate*, unsigned long)::{lambda(v8::FunctionCallbackInfo<v8::Value> const&)#1}::_FUN(v8::FunctionCallbackInfo<v8::Value> const&)
14: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
15: 0xeaeb0c
16: 0xea7f0c
17: 0xea7e5a
18: 0x2f4f091060bb

review: Needs Fixing

Reply

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-07-01:

#

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/12/rebuild

FAILED: Continuous integration, rev:145
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/12/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/105/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/112
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/69
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial/69
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/69
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/45/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial/45
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial/45/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/45
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/45/artifact/output/*zip*/output.zip
    ABORTED: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/45/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial/45
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial/45/artifact/output/*zip*/output.zip
    ABORTED: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/45/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/45/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial/45
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial/45/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/45
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/45/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/12/rebuild

review: Needs Fixing (continuous-integration)

Reply

lp://staging/~abreu-alexandre/unity-js-scopes/expand-filter-support updated on 2016-08-17

146. By Alexandre Abreu on 2016-08-17: Update ValueSliderLabels' constructor to fit the Scopes API

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2016-08-17:

#

I updated the code to fit the js example

Reply

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-08-17:

#

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/14/rebuild

PASSED: Continuous integration, rev:146
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/14/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/396
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/402
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/317
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/317
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/317
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/247/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/247
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/247/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-unity-js-scopes-ci/14/rebuild

review: Approve (continuous-integration)

Reply

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-09-06:

#

While there are no longer any visible error messages when running the example scope from lp:~marcustomlinson/unity-js-scopes/filters_example, there are no results showing up in the scope at all.

I'm not sure where things are going wrong now. Running the example scope against lp:~marcustomlinson/unity-js-scopes/filters works fine, but obviously there are some changes here (especially documentation) that I don't have.

I wonder if it'd be wise to somewhat merge the 2?

review: Needs Fixing

Reply

unity-js-scopes

Merge lp://staging/~abreu-alexandre/unity-js-scopes/expand-filter-support into lp://staging/unity-js-scopes

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers