Skip to content

Conversation

bowrocker
Copy link

Change summary

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Core Features:

  • [ x] Have you added an explanation of what your changes do and why you'd like us to include them?
  • [ x] Have you written new tests for your core changes, as applicable?
  • [ x] Have you successfully run tests with your changes locally?

User Impact

  • [] What is the user impact of this change? This change maintains the status quo in the case where customers make no changes to their HTTPS endpoint configuration and only leverages the new functionality if the customer explicitly opts into it. The risk of impact is low because customers using the HTTPS logging endpoint will be unaffected by this change by default.

Are there any considerations that need to be addressed for release?

No breaking changes are introduced.

@philippschulte
Copy link
Member

This PR depends on fastly/go-fastly#749. The period field is not yet documented in our public docs.

@philippschulte philippschulte added the blocked A fix is dependant on a separate change/feature. label Sep 15, 2025
@bowrocker
Copy link
Author

@philippschulte The docs PR merged https://github.com/fastly/api-documentation/pull/1192 so this should be ready for review. Thanks!

@bowrocker bowrocker marked this pull request as ready for review September 16, 2025 20:45
@bowrocker bowrocker requested a review from a team as a code owner September 16, 2025 20:45
@bowrocker bowrocker requested a review from kpfleming September 16, 2025 20:45
@kpfleming
Copy link
Contributor

This cannot be effectively reviewed until there has been a go-fastly release which includes the new attribute, and we're able to confirm that the tests in the PR pass as expected.

@bowrocker
Copy link
Author

This cannot be effectively reviewed until there has been a go-fastly release which includes the new attribute, and we're able to confirm that the tests in the PR pass as expected.

@kpfleming the go-fastly PR has been merged.

@kpfleming
Copy link
Contributor

Indeed it has, that's the first step. There's also been a go-fastly release, so the next step will be to update this repository to use that new release, and then rebase this branch so it is also using that new release. With all of that done the tests in the PR should be able to run and report success.

@kpfleming
Copy link
Contributor

OK, this repository has also now been upgraded to go-fastly 12.0.0, so you can rebase this branch and address the conflicts. Once that's done the tests should pass and I'll be able to review the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked A fix is dependant on a separate change/feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants