Skip to content

adding workflow files #2950

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jagdish-15
Copy link
Contributor

@jagdish-15 jagdish-15 commented Jun 5, 2025

Pull Request

Hi @kahgoh,

This PR adds the sync automation workflow we discussed. I've tested the syncing and PR creation for filepaths, metadata, and docs on my fork successfully. However, I wasn’t able to fully test the test-sync part that raises an issue, since creating issues from a fork repo isn’t allowed.

The workflow is scheduled to run automatically on the 1st and 15th of every month at 00:00 UTC. I’ve also included a workflow_dispatch trigger so you can manually run it on the main repo to verify that the issue creation for unsynced tests works as expected, as I currently can't trigger the workflow from my side on the main repo.


Reviewer Resources:

Track Policies

@jagdish-15
Copy link
Contributor Author

I believe the lint failure is caused by the bot's last commit, right?

@kahgoh
Copy link
Member

kahgoh commented Jun 5, 2025

Yeah, the lint errors are from the markdown lint upgrade. If you want to fix them, I'm happy for you raise a separate PR to fix them.

Copy link
Member

@kahgoh kahgoh left a comment

Choose a reason for hiding this comment

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

Thanks @jagdish-15! I just had a look through. It looks good. I'm planning to give it a test over the next few days.

@jagdish-15
Copy link
Contributor Author

Yeah, the lint errors are from the markdown lint upgrade. If you want to fix them, I'm happy for you raise a separate PR to fix them.

Raised the PR! Lemme know if you'd like to request any changes!

Something I'd like to mention is that the linting issues were not detected by ./gradlew check; I could only detect them by triggering the markdown workflow in my fork. Is there something I am misunderstanding?

@kahgoh
Copy link
Member

kahgoh commented Jun 6, 2025

Hey does the raise PR actions require configuring the repository to allow actions to raise and approve merge requests?

@jagdish-15
Copy link
Contributor Author

jagdish-15 commented Jun 6, 2025

It should work by default, but just to be sure, make sure this setting is enabled:

Repository Settings → Actions → General → Workflow permissions

Under Workflow permissions, select:

Read and write permissions

Still, would you mind checking it once by triggering it with workflow_dispatch?

@jagdish-15
Copy link
Contributor Author

And @kahgoh, one more thing, I've added the proposal to the forum, would you mind checking it out here

@kahgoh
Copy link
Member

kahgoh commented Jun 7, 2025

It should work by default, but just to be sure, make sure this setting is enabled:

Repository Settings → Actions → General → Workflow permissions

Under Workflow permissions, select:

Read and write permissions

Still, would you mind checking it once by triggering it with workflow_dispatch?

I got this error:

 Error: GitHub Actions is not permitted to create or approve pull requests

However, if it works after turning on the following in the Workflow permssions section:

  • Read and write permissions
  • Allow GitHub Actions to create and approve pull requests

I get this:

Error: Resource not accessible by integration

I've been testing using my fork repository.

@jagdish-15
Copy link
Contributor Author

jagdish-15 commented Jun 7, 2025

This happens because GitHub does not allow issues to be created from workflows in forked repositories. That’s also why I wasn’t able to test the issue creation part of the workflow. To test it, you'll need to run the workflow in the main repository or in a non-forked clone.

That said, I’d say give it a try in the main repo after merging the PR. I’m (admittedly) a bit biased, but I’m confident it’ll work as expected!

- name: Run configlet sync for test and capture output
id: sync_test
run: |
./bin/configlet sync --test | tee .github/auto-sync/sync-test-output.txt
Copy link
Member

@kahgoh kahgoh Jun 7, 2025

Choose a reason for hiding this comment

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

While testing, I just noticed this in the logs:

Run ./bin/configlet sync --test | tee .github/auto-sync/sync-test-output.txt
  ./bin/configlet sync --test | tee .github/auto-sync/sync-test-output.txt
  echo "output<<EOF" >> $GITHUB_OUTPUT
  cat .github/auto-sync/sync-test-output.txt >> $GITHUB_OUTPUT
  echo "EOF" >> $GITHUB_OUTPUT
  shell: /usr/bin/bash -e {0}
  env:
    PULL_REQUEST_NUMBER: 1
Error: invalid option: '--test'

I think it should be --tests (plural, not the singular --test), but we also need to choose an option between include and exclude (see configlet --help or its documentation for more details).

Copy link
Contributor Author

@jagdish-15 jagdish-15 Jun 7, 2025

Choose a reason for hiding this comment

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

Yep, that should be --tests, apologies for the mix-up. And I it by default checks all the exercises with configlet sync --tests, so we don't need to use include as we only have to get the info about unsynced tests and not actually update the test.toml files

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.

2 participants