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 validate-config action and README #20

Merged
merged 9 commits into from
Aug 5, 2024
Merged

Conversation

zkamvar
Copy link
Member

@zkamvar zkamvar commented Aug 1, 2024

This will fix #9 by implementing a validation action for config files
that additionally comments on the pull request the results of the
validation.

This will fix #9 by implementing a validation action for config files
that additionally comments on the pull request the results of the
validation.
Copy link
Member

@annakrystalli annakrystalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!!

I made some comments and suggestions for changes directly in the PR.

One last thing I forgot to mention is we also maintain details of available workflows in hubDocs. https://hubverse.io/en/latest/quickstart-hub-admin/continuous-integration.html#available-hubverse-github-actions

Any chance you can add an entry there too? (might be worth a custom PR template for this repo to remind us every time!)

validate-config/README.md Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
@zkamvar zkamvar requested a review from annakrystalli August 2, 2024 13:59
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
zkamvar added 4 commits August 2, 2024 11:54
 - all comments will have a timestamp (because it's not immediately clear from
   the edited comment)
 - added a "success" comment so that the failure comments do not hang
   around
 - added a more informative error message for workflows that don't use
   comments.
@zkamvar zkamvar requested a review from annakrystalli August 2, 2024 18:58
@zkamvar
Copy link
Member Author

zkamvar commented Aug 2, 2024

final_this_one_srsly_actually_final.docx

I added @annakrystalli's suggestions and did the following:

  1. updated checkout to v4
  2. use the r-universe instead of github for installation
  3. improved messaging to give a timestamp and to give a successful message when passing

@zkamvar
Copy link
Member Author

zkamvar commented Aug 2, 2024

Also, I have hubverse-org/hubDocs#164 in draft. It's actually ready, but I didn't want to open it until this is merged.

Copy link
Member

@annakrystalli annakrystalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehehehe almost! I made a few suggested editing to make it cleaner and more intuitive for someone wanting to override the default hub_path.

It did make me think about the workflow paths triggers though. These also need to be responsive to a change in hub_path otherwise we need to let users know to change them too right?

validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/validate-config.yaml Outdated Show resolved Hide resolved
validate-config/README.md Show resolved Hide resolved
validate-config/validate-config.yaml Show resolved Hide resolved
Comment on lines +24 to +44
For example, to validate the config of a demo hub included as part of a package you would want make sure the hub path is set to `"inst/demo_hub"`:

```diff
on:
workflow_dispatch:
pull_request:
branches: main
paths:
- - 'hub-config/**'
+ - 'inst/demo_hub/hub-config/**'
- '!**README**'

jobs:
validate-hub-config:
runs-on: ubuntu-latest
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ github.event.number }}
- HUB_PATH: ${{ github.workspace }}
+ HUB_PATH: ${{ github.workspace }}/inst/demo_hub
```
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@annakrystalli, I've modified the README to show the example. Does this seem good to you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!

Copy link
Member

@annakrystalli annakrystalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant! This looks fab now and very clear instructions. Approved!

@zkamvar zkamvar requested a review from annakrystalli August 5, 2024 14:22
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.

add an action to validate updates to hub config files
2 participants