build: add github actions

[12rambau]

the problem with pre-commit is that there are still people installing locally without them pip install -e ./ and thus pushing PR that do not respects the repository requirements. With Github actions at least you're notified.

I copy what was in the pre-commit:

• black formatter
• flake8 check

I also added:

• pytest run (colorised :smile:)

do you think it could make sense to add a codecov report ?

Also requirements are not set for the repository, could you add a requirements.txt file that could be used in the tests ?

[12rambau]

The build doesn't seem to complete successfully: https://github.com/12rambau/ipyvue/actions. This can be solved by installing Jupyter lab 3.

• [x] Would it make sense to add a requirements.txt file for the build ?

flake8 and black can be run by pre-commit: pre-commit run --all-files. This ensures we use the same versions as in the pre-commit hook.

This I know it's just that on one of my repository I saw people going around by creating strange installation

package-lock.json should not change for this PR

• [x] I don't know why and when the package-lock.json changed but I'll reverse the changes

[mariobuikhuizen]

Thanks for the changes!

I see a few issues:

• package-lock.json still has a change. You can use git checkout origin/master -- js/package-lock.json to get the file in the original state.
• .gitignore is missing a newline at the end of the file
• Can we move requirements.txt to .github or rename it to requirements_dev.txt? Jupyterlab is not always a dependency of this library (e.g. if the user only runs Voila or if the kernel is in a different environment), also pytest and coverage are not required to use this library.
• This PR has a merge conflict (probably because of the commit: 'Merge branch ....').

I like the black badge!

flake8 and black can be run by pre-commit: pre-commit run --all-files. This ensures we use the same versions as in the pre-commit hook.

This I know it's just that on one of my repository I saw people going around by creating strange installation

I don't understand from this text why we shouldn't use pre-commit run --all-files for the linting , so we don't have potential linting inconsistencies when versions of the linters don't match between dev and CI.

[12rambau]

I don't understand from this text why we shouldn't use pre-commit run --all-files for the linting , so we don't have potential linting inconsistencies when versions of the linters don't match between dev and CI.

My bad I didn't understand the question in the first place. Using the github actions instead of pre-commit, it will display the errors as PR annotation when pre-commit will just display it in the build report.

flak8 is still using the same .flake8 parameter file and black is "uncompromising" so I won't be afraid of inconsistencies :smile:

looking at the files I don't see any merge conflict and according to github (at least from my side) the branch "has no conflict with the base branch".

[mariobuikhuizen]

My bad I didn't understand the question in the first place. Using the github actions instead of pre-commit, it will display the errors as PR annotation when pre-commit will just display it in the build report.

Or my bad 😄, I meant running pre-commit run --all-files in the github action. Something like this in unit.yml:

...
 - name: Linting
   run: pre-commit install && pre-commit run --all-files
...

looking at the files I don't see any merge conflict and according to github (at least from my side) the branch "has no conflict with the base branch".

I also don't quite understand why, but I see this in this PR:

Also, If I do a checkout this branch and try to rebase, I get the following:

$ git rebase upstream/master
Auto-merging .github/workflows/unit.yml
CONFLICT (content): Merge conflict in .github/workflows/unit.yml
error: could not apply cb0b18b... build: use the dev installation
Resolve all conflicts manually, mark them as resolved with
...

With the conflict being:

...
      - name: Install dependencies
<<<<<<< HEAD
        run: pip install -r requirements.txt
=======
        run: pip install .[dev]
>>>>>>> cb0b18b (build: use the dev installation)
      - name: Test formatting
...

[12rambau]

This is black magic !

The problem is that the issue you saw was corrected in the merge. My guess is that I made a mistake between https://github.com/mariobuikhuizen/ipyvue/pull/54/commits/876dd22fea47be139e834b156dcdbcc3a56c8a6a and https://github.com/mariobuikhuizen/ipyvue/pull/54/commits/fea8c169bc16dd95bdb73eb40498cd1054750a35 as I renamed .github to github. Maybe the legacy conflict propagated.

I opened a ticket so that Github can at least explain why we are not seeing the same thing: https://support.github.com/ticket/personal/0/1419769

[12rambau]

@mariobuikhuizen I received a message from Github, apparently my fork was out of sync with your repo for a while causing problem when i tried to merge.

They told me that the situation was resolved, can you confirm ?

[mariobuikhuizen]

Nope 😞

[mariobuikhuizen]

oh .. I didn't notice it was on rebase and merge. Squash and merge works:

Sorry 😅

[12rambau]

hooray !

Is there anything you still want me to modify ?

I meant running pre-commit run --all-files in the github action

For this one, THis is what I understand. If you run pre-commit in the actions then the issues encountered by black and flake8 will be reported in the terminal. With the github actions I use, they will be reported as comments directly in the PRs/code