Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

refreshed CI settings #1395

Merged
merged 3 commits into from
Aug 3, 2022
Merged

refreshed CI settings #1395

merged 3 commits into from
Aug 3, 2022

Conversation

moljac
Copy link
Member

@moljac moljac commented Jul 28, 2022

  • bumped several nuget versions
  • matching builds for Xamarin.Android

@moljac moljac requested a review from jonathanpeppers July 28, 2022 08:35
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Does this also need to update the android workload?

- pwsh: |
dotnet workload install android
displayName: install .NET 6 Android Workload
condition: ne(variables['System.JobName'], 'linux')

@moljac
Copy link
Member Author

moljac commented Jul 29, 2022

Does this also need to update the android workload?

- pwsh: |
dotnet workload install android
displayName: install .NET 6 Android Workload
condition: ne(variables['System.JobName'], 'linux')

Not sure. But I think it makes sense to update.

@moljac moljac requested a review from jonathanpeppers July 29, 2022 12:15
@moljac
Copy link
Member Author

moljac commented Jul 29, 2022

Added snippet for workload installation and update from AX and/or GPS-FB-MLKit

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Like we did in AndroidX / GPS, can we "spot check" some of the .nupkg files to make sure the file sizes are the same?

@moljac
Copy link
Member Author

moljac commented Aug 1, 2022

Like we did in AndroidX / GPS, can we "spot check" some of the .nupkg files to make sure the file sizes are the same?

this would be quite difficult for XC repo. it builds according to manifest only folder involved in the PR. So if TensorFlow was updated it would build only TensorFlow.

Maybe closing this PR w/o merging?

@jonathanpeppers
Copy link
Member

@moljac can you make a small change in Tensorflow to test at least one of them?

@moljac
Copy link
Member Author

moljac commented Aug 1, 2022

@moljac can you make a small change in Tensorflow to test at least one of them?

OK
I will bind the same version as in GPS-FB-MLKit, so that we have one parameter less to take into account.

Should I multitarget net6.0?

We left it here as monoandroid9.0

https://github.com/xamarin/XamarinComponents/blob/main/XPlat/TensorFlow.Lite/source/Xamarin.TensorFlow.Lite.Bindings.XamarinAndroid/Xamarin.TensorFlow.Lite.Bindings.XamarinAndroid.csproj#L12

@jonathanpeppers
Copy link
Member

@moljac can you pick one that targets .NET 6? Maybe Xamarin.Android.Glide?

@moljac moljac requested a review from jonathanpeppers August 3, 2022 08:59
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

This one looks fine now, but does #1398 replace it? It seems like that one also updates the Android workload.

@moljac moljac merged commit dda722f into main Aug 3, 2022
@moljac moljac deleted the ci-refresh-20220728 branch August 3, 2022 13:32
@jpobst
Copy link
Contributor

jpobst commented Oct 13, 2022

One thing to keep in mind when changing .yaml files for this repository is that it is set up to always use the YAML from main:
https://github.com/xamarin/XamarinComponents/blob/main/azure-pipelines.yml#L15-L19

Therefore it will never test your PR changes to YAML files. (I do not know why it is set up this way 🤷‍♂️)

This commit breaks the Linux lane with:

Workload ID maui isn't supported on this platform.

As a result we can no longer publish anything out of this repository. We need to figure out if these changes were actually needed, and if so, how can we work around it for Linux.

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

Successfully merging this pull request may close these issues.

3 participants