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

[MailKit] Introducing in Xcode13 Beta 5 #12405

Merged
merged 6 commits into from
Aug 17, 2021

Conversation

tj-devel709
Copy link
Contributor

@tj-devel709 tj-devel709 commented Aug 10, 2021

I have a few comments/questions listed down below that I would like help with!

@tj-devel709 tj-devel709 added the note-highlight Worth calling out specifically in release notes label Aug 10, 2021
@tj-devel709 tj-devel709 added this to the xcode13.0 milestone Aug 10, 2021
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/frameworks.sources Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Outdated Show resolved Hide resolved
NSDate DateReceived { get; }

[NullAllowed, Export ("headers", ArgumentSemantic.Copy)]
NSDictionary<NSString, NSArray<NSString>> Headers { get; }
Copy link
Member

Choose a reason for hiding this comment

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

Can this be a smart dictionary ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, what do you mean by smart dictionary?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It might be the case it can't since headers can be anything in the mail protocol, so lets ignore it for now

[Advice ("'MEMessageActionDecision' is not available in UIKit on macOS.")]
[BaseType (typeof (NSObject))]
[DisableDefaultCtor]
interface MEMessageActionDecision : NSSecureCoding
Copy link
Member

Choose a reason for hiding this comment

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

Static class? All methods/properties are static and we have no contractor.

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 can make it a static class!
Question though, if they introduce a constructor later though and non-static methods, would changing it back be a breaking change or is that very unlikely to happen?

Copy link
Member

Choose a reason for hiding this comment

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

This can't be a static class heh

Copy link
Member

Choose a reason for hiding this comment

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

And it should not, i'm wrong we get it from https://developer.apple.com/documentation/mailkit/memessageactionhandler/3783568-decideactionformessage?changes=_3&language=objc

So we need to be able to instantiate the class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I made the interface static, I got these errors in the build: https://gist.github.com/tj-devel709/a4500a4835c99831d0d2f7883f92fe5d

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry didn't refresh before I posted that ^

src/mailkit.cs Outdated Show resolved Hide resolved
@@ -264,6 +264,7 @@ public Framework Find (string framework)
{ "Chip", "CHIP", 12, 0 },
{ "MetricKit", 12, 0 },
{ "Phase", "PHASE", 12, 0 },
{ "MailKit", "MailKit", 12, 0 },
Copy link
Member

Choose a reason for hiding this comment

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

Alphabetical order please.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

2 tests failed, 93 tests passed.

Failed tests

  • introspection/Mac Modern/Debug: Failed (Test run failed.
    Tests run: 37 Passed: 35 Inconclusive: 0 Failed: 1 Ignored: 1)
  • introspection/Mac Catalyst/Debug [dotnet]: TimedOut (Execution timed out after 1200 seconds.
    No test log file was produced)

Pipeline on Agent XAMBOT-1101.BigSur'
Merge 9bf2924 into a20d417

src/mailkit.cs Outdated Show resolved Hide resolved
src/mailkit.cs Show resolved Hide resolved
@tj-devel709
Copy link
Contributor Author

Beta5 changes need to be added here

@tj-devel709 tj-devel709 changed the title [MailKit] Introducing in Xcode13 Beta 2 [MailKit] Introducing in Xcode13 Beta 5 Aug 11, 2021
@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

2 tests failed, 95 tests passed.

Failed tests

  • introspection/Mac Catalyst/Debug [dotnet]: TimedOut (Execution timed out after 1200 seconds.
    No test log file was produced)
  • monotouch-test/tvOS - simulator/Debug: Failed

Pipeline on Agent XAMBOT-1104.BigSur'
Merge 7e3e2d4 into 2972e1b

@tj-devel709
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

ℹ️ API Diff (from PR only) (please review changes)
ℹ️ Generator Diff (please review changes)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

2 tests failed, 100 tests passed.

Failed tests

  • monotouch-test/Mac Catalyst/Debug [dotnet]: Failed (Tests run: 2638 Passed: 2489 Inconclusive: 35 Failed: 1 Ignored: 148)
  • monotouch-test/watchOS 32-bits - simulator/Debug: Failed

Pipeline on Agent XAMBOT-1101.BigSur'
Merge 4fd7b1a into cba3002

@tj-devel709
Copy link
Contributor Author

Failing tests are unrelated networking issues!

@tj-devel709 tj-devel709 merged commit 4469f6b into xamarin:main Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
note-highlight Worth calling out specifically in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants