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

CI updates #25

Merged
merged 6 commits into from
Jan 23, 2023
Merged

CI updates #25

merged 6 commits into from
Jan 23, 2023

Conversation

AhmetCanSolak
Copy link
Contributor

This PR addresses the concerns raised in #24 so far and will not be merged before discussion is finalized.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Jan 18, 2023

According to discussion in #17, I bumped test range to 3.9-3.11 in 7d867bc.

Edit: fixed.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Jan 19, 2023

@AhmetCanSolak Can you rebase to main and re-run the jobs so it's easier to fix offending lines?

Also I wonder if there is a way of silencing the doc string line length issue. #14 took care of the code formatting, but fixing doc string line breaks will take manual effort.

@AhmetCanSolak
Copy link
Contributor Author

I updated the branch and added the required black and isort fixes @ziw-liu .
There is more than line length issues on the linting side and we can address it in a separate PR.

Let me know if you are happy with proposed changes on the action tasks, so accordingly we can merge and move forward with other PRs.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Jan 21, 2023

@AhmetCanSolak Can you rebase to main and see that the pytest job passes?

Not a big deal, but it would also be nice to have the coverage job to display results in PR.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Jan 21, 2023

@AhmetCanSolak Also want to confirm that we don't need to edit pyproject.toml (i.e. both your formatting and the CI runs were based on the existing config).

@AhmetCanSolak
Copy link
Contributor Author

@AhmetCanSolak Can you rebase to main and see that the pytest job passes?

Not a big deal, but it would also be nice to have the coverage job to display results in PR.

Lint task is failing and that cancels any task that depends on it. I introduced this change intentionally to avoid wasting action hours, I will merge this PR and make a follow-up for addressing linting issues.

@AhmetCanSolak AhmetCanSolak merged commit f58d23b into czbiohub-sf:main Jan 23, 2023
@AhmetCanSolak AhmetCanSolak deleted the ci-updates branch January 23, 2023 16:46
@ziw-liu ziw-liu mentioned this pull request Feb 1, 2023
@ziw-liu
Copy link
Collaborator

ziw-liu commented Feb 2, 2023

I will merge this PR and make a follow-up for addressing linting issues.

Hi @AhmetCanSolak, have you started with this? I'm preparing to rebase #31.

@AhmetCanSolak
Copy link
Contributor Author

I will merge this PR and make a follow-up for addressing linting issues.

Hi @AhmetCanSolak, have you started with this? I'm preparing to rebase #31.

I haven't got to it yet @ziw-liu , I can probably do that next week, if you think you can move it before that go for it.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Feb 2, 2023

@AhmetCanSolak Thanks for letting me know!
I was lazy so instead of a separate PR I just started working on it in #31...

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