-
Notifications
You must be signed in to change notification settings - Fork 0
Improve Installation Process and Resolve Setup Issues #1
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
base: master
Are you sure you want to change the base?
Conversation
| ] | ||
|
|
||
| [tool.hatch.build.targets.wheel] | ||
| packages = ["schemadiff"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐼 I think there should be a empty line at the bottom of this file? Do we need to setup some linting ect for this repo now we're officially forking it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, fair point. Since this fork is just a temporary workaround (for this change) and we expect to move back to the original repo in the mid-term, and we're also thinking about dropping the dependency in the long term, setting up the whole machinery feels a bit much for now. We should definitely do it if we end up touching more in this fork later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, makes sense given the context which I didn't know before 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to modify some of the metadata in this file eg the version number, author, homepage ect to avoid confusion with the forked repo
9d7c5ff to
369496d
Compare
|
Hey @CharlotteDodd, 🖊️ The last commit it's just automatic fixes from ruff. |
e12b89d to
27cbbe5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with aligning the repo to our standards, though don't think it’s strictly needed. There's also a small downside: merge conflict hell if we ever end up merging upstream changes but still need to keep the fork.
Happy for you to merge this, but also fine to stick with just the first commit and the metadata change.
pyproject.toml
Outdated
| description = "Compare GraphQL Schemas" | ||
| readme = "README.md" | ||
| version = "1.2.4" | ||
| version = "1.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With Charlotte having fixed the cache pollution issue, we probably don't need proper versioning here if we only include this as a git dependency in theory, but probably cleaner to do it anyway. The version should increase though, as 1.0.0 might collide with an old version. Something like 1.2.4-kraken1 or so might be appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 going to use your suggestion
| version = "1.0.0" | ||
| authors = [ | ||
| { name = "Nahuel Ambrosini", email = "[email protected]" } | ||
| { name = "Mattia Baldari", email = "[email protected]" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐼 I know it's only temporary and not really important, but you'd usually keep the original author in here. authors is a list after all 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really used to this, good you told me, didn't mean to be rude 😟
f23cf74 to
987ff37
Compare
Update pyproject.toml with octoenergy-specific metadata: - Change version from 1.2.4 to 1.0.0 (fresh start for fork) - Update author to Mattia Baldari <[email protected]> - Update homepage URL to https://github.com/octoenergy/graphql-schema-diff/ This distinguishes the fork from the original Ambro17/graphql-schema-diff repository and reflects octoenergy's ownership and maintenance.
- Add ruff.toml with configuration based on octoenergy/kraken-core - Replace flake8 with ruff in dev dependencies - Remove obsolete .flake8 configuration file
Configure pre-commit with: - Ruff linting and formatting - Standard checks (trailing whitespace, EOF, AST validation, etc.) Pre-commit hooks help catch issues before committing and ensure consistent code quality across contributors.
Add CI workflow that runs on push to master and on pull requests: - Runs ruff linter - Checks code formatting with ruff format This ensures code quality checks run even for contributors who haven't installed pre-commit hooks locally.
Add new 'Development Setup' section to README with: - Instructions for setting up local development environment - How to install and use pre-commit hooks - Commands for running linting checks manually This helps new contributors get started quickly and understand the project's development workflow.
987ff37 to
18a8216
Compare
This pull request introduces a significant update to the project's development workflow, code style enforcement, and documentation. The most important changes are the migration from
flake8torufffor linting and formatting, the addition of pre-commit hooks and CI integration for code quality, and improvements to code style and documentation. These changes help standardize development practices, improve code readability, and ensure consistent enforcement of quality checks.Development workflow and tooling:
.pre-commit-config.yamlto enable pre-commit hooks for linting, formatting, and various code quality checks usingruffand other tools..github/workflows/lint.yml) to runrufflinter and formatter on every push and pull request tomaster, ensuring CI-based code quality enforcement.ruff.tomlto configure linting and formatting rules, including target Python version, line length, exclusions, and specific rule ignores..flake8configuration, fully migrating linting responsibilities toruff.Documentation and setup:
README.mdwith detailed development setup instructions, including cloning, installing dev dependencies, and using pre-commit andrufffor local checks.Project metadata and dependencies:
pyproject.tomlto reflect new project ownership, repository URL, versioning, and switched optional dev dependencies fromflake8toruffandpre-commit. Also added wheel build configuration. [1] [2]Code style and minor refactoring:
schemadiff/__init__.py,schemadiff/__main__.py,schemadiff/allow_list.py,schemadiff/changes/__init__.py), including import sorting, docstring formatting, type hinting, and argument formatting to align with modern Python best practices. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]