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

Make CI annotate pull requests with links to the {section} specified in "specification" #728

Closed
suprith-hub opened this issue Apr 7, 2024 · 11 comments · Fixed by #746
Closed

Comments

@suprith-hub
Copy link
Contributor

suprith-hub commented Apr 7, 2024

sections here refer to sections of core, validation of jsonschema, rfcXXX, ecma etc..

Come up with something for annotating the review with a link to the section.

Probably custom logic for each specification which knows what the right URL is given what we have put in the corresponding field, yes, which the simplest version is just "a base URI".

But exactly right. Some CI script for that, which annotates PRs, and warns for broken links.

Reference to this conversation:
PR #726

@suprith-hub
Copy link
Contributor Author

suprith-hub commented Apr 10, 2024

@gregsdennis @Julian I am trying to run unittest, by using:
python .\bin\jsonschema_suite check this command,
I am not able to understand why it gives failures and errors. I am too struck in this. Can I get help from anyone in resolving tests, so that I can proceed.
Because of many errors I am not able to see how the annotations are being collected

@Vinit-Pandit
Copy link
Contributor

@Era-cell just run .bin\jsonschema_suit check , there is already python env present in bin

@Vinit-Pandit
Copy link
Contributor

And to help with your error failures, please provide more details about it.

@suprith-hub
Copy link
Contributor Author

Yeah I am sharing you teh error I got:
https://elmah.io/tools/stack-trace-formatter/f5a9f5ccb7ed45e3a0450b4a863c1757/
its much longer, it would be great if you can help

@Julian
Copy link
Member

Julian commented Apr 21, 2024

That sounds like you have a quite old version of the python-jsonschema implementation I maintain which is used to run sanity checks on the suite's test cases. You could run pip install -U jsonschema, or you could use tox and run that which will always give you the right version, but none of that has anything to do with this issue so not 100% sure what you're trying to do! This issue was about creating a GitHub action workflow to comment on pull requests, it doesn't involve any of that stuff.

@suprith-hub
Copy link
Contributor Author

Hey! It works now except the UnicodeDecodeError.
And I thought the annotations have to be logged by using print() in tests as I wasn't aware that its possible in workflows. Dumb me.
It seems interesting to create a workflow. But I guess I will need authorization for that..

@Julian
Copy link
Member

Julian commented Apr 21, 2024

You should be able to get it working on a fork of yours, then send a PR for review!

@suprith-hub
Copy link
Contributor Author

You should be able to get it working on a fork of yours, then send a PR for review!

Ohh! I see, I get it now. Thank you ❤️‍🔥

@suprith-hub
Copy link
Contributor Author

suprith-hub commented Apr 23, 2024

I am asking this before I work on this technique completely:
Can I write a python script like this which will be called by a .github/workflow/new_workflow
new_workflow will trigger on pull_request making changes on tests/** folder

from github import Github
import os
import sys

def main():
    # GitHub authentication using personal access token
    # Replace 'YOUR_PERSONAL_ACCESS_TOKEN' with your actual token
    g = Github(os.environ.get('GITHUB_TOKEN'))

    # Get repository and pull request number from environment variables
    repo_name = os.environ.get('GITHUB_REPOSITORY')
    pull_request_number = os.environ.get('GITHUB_REF').split('/')[-1]

    if not repo_name or not pull_request_number:
        print("Repository name or pull request number not found in environment variables.")
        sys.exit(1)

    # Get repository object
    repo = g.get_repo(repo_name)

    # Get the pull request object
    pr = repo.get_pull(int(pull_request_number))

    # Get the list of changed files in the pull request
    changed_files = [file.filename for file in pr.get_files()]

    print(changed_files)
    # Filter the list to include only JSON files in the 'tests' directory
    # changed_json_files = [file for file in changed_files if file.startswith('tests/') and file.endswith('.json')]

    # if changed_json_files:
    #     # Commit and push changes
    #     commit_message = "Update JSON files"
    #     commit_and_push_changes(repo, pr.base.ref, commit_message)
    # else:
    #     print("No changes detected in JSON files.")

if __name__ == "__main__":
    main()

So, can I use this method?

@Julian
Copy link
Member

Julian commented Apr 24, 2024

The format you need to output is the format needed for GitHub Actions annotations -- see e.g. here -- it's a sort of funny looking string starting with ::. So you should print a string like that for each line containing a specification link.

I would recommend starting by completely ignoring git, in other words simply output annotations for all specification links! I think it's possible GitHub will then even automatically handle filtering showing only the ones which are relevant for a diff on a PR. I.e. if you spit out everything, GitHub will only render the ones for lines changed in the PR. It's been awhile since I looked at this API / docs, but yeah I'd start there, and if need be we can do the filtering afterwards if it turns out not to happen automatically, but undoubtedly this will be a very common thing that anyone using these will have needed to do.

Lemme know if that helps. (And thanks again!)

@suprith-hub
Copy link
Contributor Author

I felt this easy, it was time taking but I have raised the PR, I hope this is what is expected.
But yeah I understood partially what you said 😅

@Julian Julian linked a pull request May 9, 2024 that will close this issue
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 a pull request may close this issue.

3 participants