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

Allow user to specify octodns ref to run #53

Merged
merged 15 commits into from
May 9, 2021
Merged

Allow user to specify octodns ref to run #53

merged 15 commits into from
May 9, 2021

Conversation

solvaholic
Copy link
Owner

@solvaholic solvaholic commented May 3, 2021

Description

Motivation and Context

To facilitate the request in #50, primarily.

I think moving from the Docker image to the runner scripts gives better flexibility and extensibility, and makes this action a much less complex addition to a workflow.

How Has This Been Tested?

No testing, yet. Manually, running this action @issue50 from repos holding configs.

Screenshots (if appropriate):

Types of changes

  • 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:

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.

Changes

  • Add changelog entry
  • Update README
  • Remove files that were only there to support using Docker image and container
  • Rewrite action.yml to use scripts rather than container
  • Create scripts
  • Remove dockerlint from linter.yml

[] Update release.yml and docs/release.md (goto #55)

solvaholic added 7 commits May 2, 2021 20:33
and restore the corresponding octodns-sync.sh, like in 4bca04e.
on the runner rather than in a Docker container.

Removing the container image will simplify this project.

'lint' is now the only Makefile target.
and two more Docker-related files I missed earlier.
and catch up README.md
@solvaholic solvaholic self-assigned this May 3, 2021
@solvaholic solvaholic marked this pull request as draft May 3, 2021 22:27
and add the PR number to the changelog stub
@solvaholic solvaholic marked this pull request as ready for review May 4, 2021 18:04
@solvaholic solvaholic added the enhancement New feature or request label May 7, 2021
@solvaholic
Copy link
Owner Author

Using this syntax I demonstrated the current version of this action a) runs and b) will update the pull request as-configured:

      - name: Test current configuration
        id: octodns-test
        uses: solvaholic/octodns-sync@issue50
        with:
          config_path: test-config.yaml
          doit: 'no'
          add_pr_comment: 'Yes'
          octodns_ref: ${{ steps.pick-ref.outputs.octodns_ref }}
          pr_comment_token: ${{ secrets.PAT }}

So far, so good. Before merging I'll run more tests and make sure --doit works too. If that's in a public repository I'll link it here.

@solvaholic
Copy link
Owner Author

--doit tested OK in a private repo, tho #51 freaks me out every time I notice it happening. Also spotted #57 while testing.

I'll go ahead and make v2.1.3 from be7cf5b, then merge this PR to update main.

#57 cannot impact anyone currently using main, since the octodns_ref input doesn't exist yet there. It can only hurt when octodns_ref is set to a branch name - if octodns_ref matches a version tag install.sh will use pip install which will avoid the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support using unreleased versions of octodns
1 participant