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

Adding earl report for rdfjs-c14n #80

Merged
merged 2 commits into from
Feb 12, 2023
Merged

Adding earl report for rdfjs-c14n #80

merged 2 commits into from
Feb 12, 2023

Conversation

iherman
Copy link
Member

@iherman iherman commented Feb 11, 2023

This is just to test the reporting mechanism... my separate tester for the rdfjs-c14n algo generates now an earl report which I have just submitted. Is there anything else that I am supposed to do?

@gkellogg
Copy link
Member

It would be nice to automate the process and run the scripts to regenerate the report (and do something similar for test manifests), but I haven't had much success in using GH Action tools such as https://github.com/marketplace/actions/add-commit to automate the process of building files and automatically committing them.

For the reports, the process is described in https://github.com/w3c/rdf-canon/tree/main/reports#readme, but for now, I simply run through it and update the PR or generate a new PR with the updates.

@gkellogg
Copy link
Member

Note some syntax errors in rdfjs-c14n-report.ttl which are fixed in the update.

@jeswr
Copy link
Member

jeswr commented Feb 12, 2023

but I haven't had much success in using GH Action tools such as https://github.com/marketplace/actions/add-commit

@gkellogg You can directly use the git and gh commands in github actions if you supply a GH_TOKEN. For instance this is how I automate the creation of PRs in one of the projects I contribute to (it is part of a worflow that automatically generates, commits, pushes, and releases the WebAssembly distribution of the EYE reasoner whenever new updates are made to EYE)

@gkellogg
Copy link
Member

I’ll have to work on that; thanks for the pointer.

@iherman
Copy link
Member Author

iherman commented Feb 12, 2023

For the reports, the process is described in https://github.com/w3c/rdf-canon/tree/main/reports#readme, but for now, I simply run through it and update the PR or generate a new PR with the updates.

I have created an action to generate reports locally in another repo, see:

https://github.com/w3c/epub-tests/blob/main/.github/workflows/generate.yml

It runs some local scripts and commits the results. It does the job, although it is not 100% smooth: if the underlying script is changed in the repo, then there are some false runs with error reports that I never really chased down. But it may still give you some inspiration...

Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

Approving as we should certainly accept Ivan's report from his implementation. I see this PR now has some other general fixes in it from Gregg too, which is fine. I didn't go through those line by line, there's a lot here. :)

@gkellogg
Copy link
Member

The “other fixes”are just the result of running the tools. I’ll spend time on automating this shortly.

@gkellogg gkellogg merged commit 1eec9a0 into main Feb 12, 2023
@gkellogg gkellogg deleted the rdfjs-c14n-earl-report branch February 12, 2023 17:25
@gkellogg
Copy link
Member

@jeswr I could use some help on this.

@gkellogg You can directly use the git and gh commands in github actions if you supply a GH_TOKEN. For instance this is how I automate the creation of PRs in one of the projects I contribute to (it is part of a worflow that automatically generates, commits, pushes, and releases the WebAssembly distribution of the EYE reasoner whenever new updates are made to EYE)

I forked the repo, and created a workflow here which is part of gkellogg#1 after generating GH_TOKEN as a repository secret. I've tried several variations, but the action continues to fail. I've tried the solution suggested in actions/checkout#664 (comment), but that fails in gh pr merge $branch --auto --merge saying that permission is died (log here).

Any help would be appreciated. It would be very useful to be able to do this here, and in other repositories, but I keep stumbling on some of these issues.

(@iherman, your process seems to update "main" after a PR is merged, while this attempts to add commits to the branch being merged).

@jeswr
Copy link
Member

jeswr commented Feb 15, 2023

but that fails in gh pr merge $branch --auto --merge saying that permission is died (log here).

That is because you don't give a GH_TOKEN defined in the environment for that last step. Try adding the following at the end of the last step https://github.com/eyereasoner/eye-js/blob/41965b4eaa4c60679159702dee293edf29cd8514/.github/workflows/update-eye.yml#L51-L52

@jeswr
Copy link
Member

jeswr commented Feb 15, 2023

Also be warned; only use gh pr merge $branch --auto --merge if you actually want to enable automerge (which will automatically merge the branch you have created into main once any required CI checks you have are passing).

For this to work you will need to have automerge enabled in the repository settings and also add any branch protections you want in place to gate automerging (i.e. if you have any gating CI checks mark them as required in your branch protection settings for the main branch).

@gkellogg
Copy link
Member

What I'm hoping to do is to create a branch for the commits containing the auto update and merge it into the branch used for the PR. We don't want to generally enable automatically merging to main.

But, I admit, I'm out of my depth on this particular usage pattern in GH Actions. It would be nice to have the effects of the automatic builds added to the branch used for the PR which triggers it, but I could see how that could get into an infinite loop. Perhaps @iherman's strategy of only merging to main when a PR is merged is better? But, then you won't be able to see the effects of the auto update before merging the PR.

@jeswr
Copy link
Member

jeswr commented Feb 16, 2023

We don't want to generally enable automatically merging to main.

Ok - in that case you need to make sure to set the target branch in the previous line gh pr create -t "$msg" -b "$msg" -B ${{ github.base_ref }} otherwise it will target the main branch

@jeswr
Copy link
Member

jeswr commented Feb 16, 2023

but I could see how that could get into an infinite loop.

You could add an if condition to the script which checks the last commit and only continues if the author of the last commit was not the report generation bot

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.

4 participants