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

Fix broken Pre-commit #12

Open
wants to merge 16 commits into
base: int_cast_aboslute_k_debug
Choose a base branch
from
Open

Conversation

twicki
Copy link
Owner

@twicki twicki commented Dec 4, 2024

Description

Describe the content of the PR and links to related issues, bugs of features.

Requirements

Before submitting this PR, please make sure:

  • The code builds cleanly without new errors or warnings
  • The code passes all the existing tests
  • If this PR adds a new feature, new tests have been added to test these
    new features
  • All relevant documentation has been updated or added

Additionally, if this PR contains code authored by new contributors:

  • All the authors are covered by a valid contributor assignment agreement,
    signed by the employer if needed, provided to ETH Zurich
  • The names of all the new contributors have been added to an updated
    version of the AUTHORS.rst file included in the PR

Copy link
Collaborator

@romanc romanc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

A suggestion: Since we are on that debug branch of your fork for some time, it might be worth to have CI run on that branch to catch things like these before they happen.

@romanc
Copy link
Collaborator

romanc commented Dec 4, 2024

Given that enabling all of CI is problematic with the fast-moving pace (and deliberate "let's not implement it in all the backends" decision), how about we only enable the "code quality" workflow? That way tests won't get in our way while we are sure that linting & formatting passes. For me, this would be a good compromise between dev speed on an unstable branch and enforced consistency through CI.

@FlorianDeconinck
Copy link
Collaborator

I think this unearthed actual issues though. @twicki will look through first - and we might do a clever partial deactivation. We can log in GEOS-ESM/SMT-Nebulae#98 any "reactivation" needed before we start moving back all of those features in mainline

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.

3 participants