Add Tizen backend #1546

rookiejava · 2017-12-07T05:12:44Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature

What is the current behavior? (You can also link to an open issue here)
There is no tizen as a backend platfom of ReactiveUI

What is the new behavior (if this is a feature change)?
Tizen has been added as a backend platform of ReactiveUI

What might this PR break?
None

Please check if the PR fulfills these requirements

Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

Other information:
This PR is a latest revision of #1387. And, this PR contains only the minumum requrirements to support Tizen. So, implementations that need to work correctly (like android, iOS), should be required through different PRs.

coveralls · 2017-12-07T05:19:50Z

Coverage increased (+0.03%) to 69.619% when pulling 8beb63a on rookiejava:add-tizen into a20157d on reactiveui:develop.

ghuntley · 2017-12-07T07:51:07Z

Howdy @rookiejava thank-you so much for your contribution. I've noticed on your github profile that you have done some work on the tizen platform before. How well do you know the platform? Do you work at Samsung? We are looking for a platform lead. I'm happy to work with you, pair with you and video call with you to discuss the broader picture. Last I was aware was that .net ++ tizen was immature and it wasn't possible to actually target the platform with nuget (even though the tfm is defined) - has this changed?

rookiejava · 2017-12-07T23:54:10Z

@ghuntley Yes. The latest vesrion of Tizen (Tizen 4.0 M2), that offially supports .NET, was released in the last month. For more details, see https://developer.tizen.org/tizen/release-notes/tizen-4.0-public-m2.

coveralls · 2017-12-18T00:27:30Z

Coverage increased (+0.03%) to 69.619% when pulling 3201e27 on rookiejava:add-tizen into a20157d on reactiveui:develop.

ghuntley · 2018-05-11T12:25:25Z

@rookiejava and @ideallee we are looking for two people (you?!) to join the core team of ReactiveUI and help shape the future of Tizen. See https://reactiveui.net/team/ for specifics and if this is of interest to please join us over in https://reactiveui.net/slack at https://reactivex.slack.com/archives/CAK67UA7M/p1526040674000046

Tizen is something we really want to add in Splat, ReactiveUI and Akavache. Let's do it (soon - like next release?)

Team
ReactiveUI is a composable, cross-platform model-view-viewmodel framework for all .NET platforms that is inspired by functional reactive programming which is a paradigm that allows you to express the idea around a feature in one readable place, abstract mutable state away from your user interfaces and improve improve the testability of your application.

Create Account | Slack

Slack

ghuntley · 2018-05-17T12:48:40Z

This branch has been updated and is almost ready for merge. I'm having some difficulties with EventGenerator and tizen. Any help would be appreciated. Parking this for now.

Repro:

./build.cmd

src/EventBuilder/ is the code generator for src/ReactiveUI.Events/Events_TIZEN.cs

rookiejava · 2018-05-28T09:53:12Z

@ghuntley As for this issue, I and my colleague (@WonyoungChoi) took a look inside what happend. And we found there is a bug on EventTenplateInformation class. It seems that it doesn;t work correctly in case of genereic instance type.

ReactiveUI/src/EventBuilder/Cecil/EventTemplateInformation.cs

Lines 39 to 47 in 10b7797

    
           var generic = ei.EventType as GenericInstanceType; 
        
           if (generic != null) { 
        
               foreach ( 
        
                   var kvp in 
        
                       type.GenericParameters.Zip(generic.GenericArguments, (name, actual) => new { name, actual })) { 
        
                   var realType = GetRealTypeName(kvp.actual); 
        
                   ret = ret.Replace(kvp.name.FullName, realType); 
        
               }

According to above logic, if real type name contains kvp name ('T', 'U', 'R'), it replaced into real type name. As a result, the below 2 cases replaced unintended.

IObservable<Tizen.NUI.UIComponents.Slider.MarkReachedEventArgs> MarkReached
- MarkReachedEventArgs turned into MarkBooleaneachedEventArgs
  - (R => Boolean)
Observable<Tizen.NUI.ImfManager.EventReceivedEventArgs>
- EventReceivedEventArg turned into EventTizen.NUI.ImfManager.EventeceivedEventArg
  - (R => Tizen.NUI.ImfManager.Event)

Obviously, this is not a bug on only for Tizen, but everywhere. So, I'd like to recomend

a) fix EventTemplateInformation correctly for generic instance type
b) ignore these 2 exceptional cases as for now (even if temporaily way)

rookiejava · 2018-05-28T11:39:27Z

I've tweaked it a little bit and build successfully.

olevett

One extra little change I think is worth doing, can you add to the Directory.build.targets an entry for tizen, even if it doesn't do anything at the moment?

olevett · 2018-05-30T07:51:55Z

src/EventBuilder/Cecil/EventTemplateInformation.cs

+            {"System.Boolean`1", "Boolean"},
+
+
+            {"System.EventHandler", "EventHandler"},


I'm not sure the tizen40 comment is that clear, or necessarily adds that much

Well, I did not make this change, but I've fixed unnecessary blanks and comments.

olevett · 2018-05-30T07:52:04Z

src/EventBuilder/Cecil/EventTemplateInformation.cs

+            {"Tizen.NUI.EventHandlerWithReturnType`1", "Tizen.NUI.EventHandlerWithReturnType"},
+            {"Tizen.NUI.EventHandlerWithReturnType`2", "Tizen.NUI.EventHandlerWithReturnType"},
+            {"Tizen.NUI.EventHandlerWithReturnType`3", "Tizen.NUI.EventHandlerWithReturnType"},
+


Bonus whitespace

I did not make this change, but I've fixed unnecessary blanks and comments.

olevett · 2018-05-30T07:52:20Z

src/EventBuilder/Cecil/EventTemplateInformation.cs

        };

-        private static string RenameBogusWinRTTypes(string typeName)
+        private static string RenameBogusTypes(string typeName)


olevett · 2018-05-30T07:53:13Z

src/EventBuilder/Cecil/EventTemplateInformation.cs

@@ -43,7 +65,12 @@ private static string GetEventArgsTypeForEvent(EventDefinition ei)
                        type.GenericParameters.Zip(generic.GenericArguments, (name, actual) => new { name, actual })) {
                    var realType = GetRealTypeName(kvp.actual);

-                    ret = ret.Replace(kvp.name.FullName, realType);
+                    var temp = ret.Replace(kvp.name.FullName, realType);


I'm not sure I quite get this change, shouldn't we be replacing type names for multiple generic type arguments?

This is a fix to resolve the potential issues (to avoid unintended replacement) mentioned above with minimal changes. It's nothing more, nothing less.

olevett · 2018-05-30T07:57:18Z

src/EventBuilder/Platforms/Tizen.cs

+
+            var elmSharp = Directory.GetFiles(packageUnzipPath, "ElmSharp*.dll", SearchOption.AllDirectories);
+
+            foreach (var assembly in elmSharp) {


Assemblies.AddRange(elmSharp);

I did not make this change, but I've updated it anyway.

olevett · 2018-05-30T08:03:10Z

src/EventBuilder/Platforms/Tizen.cs

+                    (exception, timeSpan, context) => {
+                        Log.Warning(
+                            "An exception was thrown whilst retrieving or installing {packageName}: {exception}",
+                            _packageName, exception);


The string interpolations here (and ln22 and ln42) need a $ at the start of the string, e.g. ln 58.

I did not make this change, but I've fixed anyway.

rookiejava · 2018-07-11T08:27:40Z

@olevett I've also added tizen to Directory.build.targets. Thank you.

rookiejava · 2018-07-12T03:33:31Z

@ghuntley could you review and merge this?

olevett · 2018-07-12T10:39:57Z

Thanks @rookiejava!

ghuntley self-assigned this Dec 7, 2017

ghuntley added the tizen label Dec 7, 2017

Add Tizen backend

3201e27

myroot force-pushed the add-tizen branch from 8beb63a to 3201e27 Compare December 18, 2017 00:21

ghuntley changed the base branch from develop to master May 17, 2018 10:17

Merge branch 'master' into add-tizen

92fd7f7

ghuntley requested review from a team May 17, 2018 10:18

red build: added events - require help finishing this

98d7aa3

rookiejava requested a review from a team May 17, 2018 12:37

ghuntley mentioned this pull request May 17, 2018

feature: add the tizen30 platform to reactiveui #1387

Closed

3 tasks

Fix unexpedted string replacement for generic intstance type

9d0a22f

Merge branch 'master' into add-tizen

eb86f67

olevett reviewed May 30, 2018

View reviewed changes

vatsalyagoel and others added 2 commits June 11, 2018 02:34

Merge branch 'master' into add-tizen

11e58eb

Fix string interpolations and code clean up

b15c737

Merge branch 'master' into add-tizen

37c5396

olevett approved these changes Jul 12, 2018

View reviewed changes

olevett merged commit e3eb8ef into reactiveui:master Jul 12, 2018

glennawatson pushed a commit that referenced this pull request Mar 23, 2019

feature: add Tizen platform support (#1546)

2862fb4

lock bot locked and limited conversation to collaborators Jun 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Tizen backend #1546

Add Tizen backend #1546

rookiejava commented Dec 7, 2017

coveralls commented Dec 7, 2017

ghuntley commented Dec 7, 2017

rookiejava commented Dec 7, 2017

coveralls commented Dec 18, 2017

ghuntley commented May 11, 2018 •

edited by unfurl-links bot

Loading

ghuntley commented May 17, 2018

rookiejava commented May 28, 2018 •

edited

Loading

rookiejava commented May 28, 2018

olevett left a comment

olevett May 30, 2018

rookiejava Jul 11, 2018

olevett May 30, 2018

rookiejava Jul 11, 2018

olevett May 30, 2018

olevett May 30, 2018

rookiejava Jul 11, 2018

olevett May 30, 2018

rookiejava Jul 11, 2018

olevett May 30, 2018

rookiejava Jul 11, 2018

rookiejava commented Jul 11, 2018

rookiejava commented Jul 12, 2018

olevett commented Jul 12, 2018

		{"System.Boolean`1", "Boolean"},


		{"System.EventHandler", "EventHandler"},


		var elmSharp = Directory.GetFiles(packageUnzipPath, "ElmSharp*.dll", SearchOption.AllDirectories);

		foreach (var assembly in elmSharp) {

Add Tizen backend #1546

Add Tizen backend #1546

Conversation

rookiejava commented Dec 7, 2017

coveralls commented Dec 7, 2017

ghuntley commented Dec 7, 2017

rookiejava commented Dec 7, 2017

coveralls commented Dec 18, 2017

ghuntley commented May 11, 2018 • edited by unfurl-links bot Loading

ghuntley commented May 17, 2018

rookiejava commented May 28, 2018 • edited Loading

rookiejava commented May 28, 2018

olevett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rookiejava commented Jul 11, 2018

rookiejava commented Jul 12, 2018

olevett commented Jul 12, 2018

ghuntley commented May 11, 2018 •

edited by unfurl-links bot

Loading

rookiejava commented May 28, 2018 •

edited

Loading