Skip to content

Conversation

@greenEkatherine
Copy link
Contributor

@greenEkatherine greenEkatherine commented Aug 4, 2021

To double check:

Addressed #7509

@riarenas
Copy link
Contributor

riarenas commented Aug 4, 2021

Do you have a test build with this validation running?

Would it be possible to add this to the PR pipeline as well and not only to the official build? It's always better to block these things in PR than after the merge.

@greenEkatherine
Copy link
Contributor Author

greenEkatherine commented Aug 4, 2021

Do you have a test build with this validation running?

Would it be possible to add this to the PR pipeline as well and not only to the official build? It's always better to block these things in PR than after the merge.

No, I will try to add it into PR pipeline

@riarenas riarenas requested a review from epananth August 4, 2021 16:00
@missymessa
Copy link
Member

What's the plan if this task is ever deprecated or breaks?

@riarenas
Copy link
Contributor

riarenas commented Aug 4, 2021

If the task breaks, the official build would catch it, which means this is adding validation to that task that is currently not present until we hit the error in arcade-validation.

If the task is deprecated we'll have to replace this with whatever we use to publish symbols, or it would be worth adding the extra test-only task.

@missymessa
Copy link
Member

it would be worth adding the extra test-only task.

This was my recommendation in the original issue :)

lukas-lansky
lukas-lansky previously approved these changes Aug 5, 2021
@greenEkatherine greenEkatherine force-pushed the validate-sdk-task-script branch from 4338725 to 72c4a3b Compare August 6, 2021 13:03
@greenEkatherine
Copy link
Contributor Author

I moved it into PR pipeline but it doesn't pass. I run locally to check the reason and it looks like PAT issue. I tried to replace it with DryRunPTA - no success. Maybe use some test "echo" project instead?

@premun
Copy link
Member

premun commented Feb 4, 2022

@greenEkatherine I guess this one can be closed?

@greenEkatherine
Copy link
Contributor Author

@greenEkatherine Katya Sokolova FTE I guess this one can be closed?

It was on FR and I lost the context. No idea, sorry.

@premun
Copy link
Member

premun commented Feb 10, 2022

Original issue was resolved by Jakub with a different PR, closing

@premun premun closed this Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants