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 CI workflow #149

Merged
merged 44 commits into from
Sep 22, 2023
Merged

Add CI workflow #149

merged 44 commits into from
Sep 22, 2023

Conversation

KacperNapierski
Copy link
Contributor

@KacperNapierski KacperNapierski commented Sep 19, 2023

PR is direct response to #145 issue. I've created workflow that test linting and builds graphenex on windows and linux automatically on PR.

Description

The CI.yaml workflow activates when new changes in folders docker and graphenex are PR'ed/pushed into main branch. Workflow can also be started manually with workflow dispatch.
Workflow contains 3 jobs:

  • Linter check - uses flake8 linter for checks. Is first job chronologically and is required for further tests to run. It's done this way to save time on building if basic test do not run successfully. I think it will be bonus after fixing existing lint problems.
  • Linux setup - prepare ubuntu 22 machine with updated packages for graphenex installation, uses workaround from Ubuntu 22.04 installation tutorial  #121 and runs graphenex
  • windows setup - install python 3.10.0, update packages, uses workaround from ImportError: cannot import name 'Mapping' from 'collections'  #127 and runs graphenex

Motivation and Context

Help with testing and stigmatization of development pipline

#145

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@jxdv
Copy link
Member

jxdv commented Sep 19, 2023

Hopefully we won't need those workarounds in the future, as we just need to replace some unmaintained libs. See: #135 newest comment.

@KacperNapierski
Copy link
Contributor Author

That would be great and probably make gX more reliable for all users. If that happens it would be small change for workflow and I will just update it then.

Copy link
Member

@orhun orhun left a comment

Choose a reason for hiding this comment

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

This looks great, just had some comments/suggestions.

.github/workflows/CI.yaml Outdated Show resolved Hide resolved
.github/workflows/CI.yaml Outdated Show resolved Hide resolved
.github/workflows/CI.yaml Outdated Show resolved Hide resolved
.github/workflows/CI.yaml Outdated Show resolved Hide resolved
.github/workflows/CI.yaml Outdated Show resolved Hide resolved
@KacperNapierski
Copy link
Contributor Author

Let's not close #145 just yet. @KacperNapierski thanks for implementing this CI workflow, but I don't see the lint issues fixed here. Are you planning another PR for that?

I know @orhun said I can make 2 seperate PRs for linter and CI. My thought process was: if CI will work and check lintering everyone can make linter changes and test them as we progress with fixes

@jxdv
Copy link
Member

jxdv commented Sep 21, 2023

Let's not close #145 just yet. @KacperNapierski thanks for implementing this CI workflow, but I don't see the lint issues fixed here. Are you planning another PR for that?

I know @orhun said I can make 2 seperate PRs for linter and CI. My thought process was: if CI will work and check lintering everyone can make linter changes and test them as we progress with fixes

Sure, makes sense.

@orhun
Copy link
Member

orhun commented Sep 22, 2023

Is this ready for re-review?

@KacperNapierski
Copy link
Contributor Author

Is this ready for re-review?

I think so, linux and windows builds with poetry from code from this repository.

.github/workflows/CI.yaml Outdated Show resolved Hide resolved
.github/workflows/CI.yaml Outdated Show resolved Hide resolved
.github/workflows/CI.yaml Show resolved Hide resolved
KacperNapierski and others added 2 commits September 22, 2023 15:29
Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
@orhun orhun self-requested a review September 22, 2023 13:32
@orhun orhun changed the title CI workflow Add CI workflow Sep 22, 2023
@orhun orhun merged commit 9be980d into grapheneX:master Sep 22, 2023
orhun added a commit that referenced this pull request Sep 22, 2023
* refactoring

* string pep reformat

* arg pep reformat

* edit doc string

* remove unused import

* Add CI workflow (#149)

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>

* Format the workflow file

* Rename workflow file

* refactoring

* string pep reformat

* arg pep reformat

* edit doc string

* remove unused import

* update exceptions

* update commands & web

* lint ignore E501

* ignore useless rules

* remove whitespaces

---------

Co-authored-by: KacperNapierski <80033411+KacperNapierski@users.noreply.github.com>
Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
@jxdv jxdv mentioned this pull request Sep 22, 2023
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.

None yet

3 participants