search

Azure / AppConfiguration-JavaScriptProvider

The configuration provider for consuming data in Azure App Configuration from JavaScript applications like Node or browser apps.
https://github.com/Azure/AppConfiguration
MIT License
7 stars 1 forks source link

fix linting error #11

Closed Eskibear closed 11 months ago

Eskibear commented 11 months ago

CI failed due to linting error after I merged #10 .

It's caused by the merging order, I merged #8 (introducing linting error) first and then #10 (introducing linting check). That's why CI of both PR are ok, but failed after merging.

Eskibear commented 11 months ago

@zhenlan @juniwang Can you help to update branch policy as below to make sure it won't happen in the future?

image
juniwang commented 11 months ago

I don't have wirte access to this repo.

zhenlan commented 11 months ago

@Eskibear are you asking to turn off that branch policy? We have that enabled for all other repos too.

Eskibear commented 11 months ago

@zhenlan No, I'm asking you to turn ON the policy. I assume that it's off for the moment.

Eskibear commented 11 months ago

Let me clarify:

CI passed for the PR #10 , but failed for main branch after merging it. That's because we merged another PR #8 into main branch, but didn't update PR #10 to re-trigger CI.

If we turn on the policy as I mentioned above, it will require to update PR #10 before merging and thus avoid this kind of issue. In such case, I expect we have additional check as below, and image

zhenlan commented 11 months ago

The Require branches to be up to date before merging is enabled already for the main branch and is still enabled now.

Eskibear commented 11 months ago

That would be strange. Anyway, it's good to know the policy is enabled. Thank you for confirming that. Let me just keep an eye on it.