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

add Position to RulesetRule #1939

Closed

Conversation

vroy
Copy link

@vroy vroy commented May 3, 2024

This will allow to update position of a specific rule in PATCH:

Description

Has your change been tested?

Screenshots (if appropriate):

Types of changes

What sort of change does your code introduce/modify?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • This change is using publicly documented in cloudflare/api-schemas
    and relies on stable APIs.

@vroy vroy requested a review from jacobbednarz as a code owner May 3, 2024 18:23
Copy link
Contributor

github-actions bot commented May 3, 2024

changelog detected ✅

@vroy vroy changed the title fix: add Position to RulesetRule add Position to RulesetRule May 3, 2024
@jacobbednarz
Copy link
Member

thanks for the PR however, the API schema documentation for position is missing so we cannot add it until that is resolved by the service team.

once that is added, it will automatically appear in 2.x.

@jacobbednarz jacobbednarz added the workflow: pending-schemas Indicates an issue or PR requires changes from an upstream library. label May 6, 2024
@jacobbednarz
Copy link
Member

apologies - i was looking at the wrong branch. this does already exist in the API schemas and the 2.x SDK. see https://github.com/cloudflare/cloudflare-go/blob/v2/rulesets/rule.go#L5341

as it exists in 2.x and isn't used elsewhere in the v0 library, i think we're fine to close this one out and recommend using 2.x instead of backporting this one.

@vroy
Copy link
Author

vroy commented May 6, 2024

Thanks, somehow missed when searching the v2 branch.

We'll certainly be looking at adopting 2.x SDK but until it becomes the main branch we're hesitant. Can this really not be backported?

Screenshot 2024-05-06 at 9 47 39 AM

@jacobbednarz
Copy link
Member

as that release note mentions, it's stable for majority of projects. we are leaving that warning in place for now as we are still cleaning up type names, etc. which are technically breaking changes and folks should be aware of it.

we have no timeline on when we switch the primary branch (nor should it matter given that is a GitHub-ism; you can use any release with confidence that it will largely work).

we have other tooling that is in the process of porting over to the new SDKs and until those are complete, the legacy SDK will remain as is today but we are not extending the current footprint except where those tools depend on the legacy SDK (see examples of recent zero trust changes for Terraform). rule.position values are not used in downstream tooling today, so we don't have plans to backport and support it.

if you really want this but also want to stick onto v0, you can run these libraries side by side. check out cloudflare/terraform-provider-cloudflare#3262 if you'd like some prior art to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow: pending-schemas Indicates an issue or PR requires changes from an upstream library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RulesetRule missing position arguments
2 participants