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

PR validation, tool config #112

Merged
merged 11 commits into from
Oct 31, 2024
4 changes: 4 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- [ ] I have read the [section on commits and pull requests](https://github.com/NaturalHistoryMuseum/ckanext-doi/blob/main/CONTRIBUTING.md#commits-and-pull-requests) in `CONTRIBUTING.md`


Describe your changes, tagging relevant issues where possible.
8 changes: 4 additions & 4 deletions .github/workflows/bump.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ on:

jobs:
bump-version:
if: "!startsWith(github.event.head_commit.message, 'bump:')"
name: Bump version and create changelog
runs-on: ubuntu-latest
name: "Bump version and create changelog"
if: "!startsWith(github.event.head_commit.message, 'bump:')"
steps:
- name: Check out
uses: actions/checkout@v3
- name: Checkout source code
uses: actions/checkout@v4
with:
token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
fetch-depth: 0
Expand Down
27 changes: 27 additions & 0 deletions .github/workflows/pull-requests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: Validate pull requests

on:
pull_request:
types: [opened, edited, reopened, synchronize]

jobs:
validate-commits:
name: Validate commit messages
runs-on: ubuntu-latest
steps:
- name: Checkout source code
uses: actions/checkout@v4
- name: Check commit message format
uses: webiny/[email protected]
with:
allowed-commit-types: 'feat,fix,refactor,perf,docs,style,test,build,ci,chore,new,patch,revert,ui'
pre-commit:
name: Run pre-commit checks
runs-on: ubuntu-latest
steps:
- name: Checkout source code
uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v5
- name: Run pre-commit
uses: pre-commit/[email protected]
9 changes: 4 additions & 5 deletions .github/workflows/pypi-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,16 @@ permissions:

jobs:
deploy:
name: Deploy package to PyPI
runs-on: ubuntu-latest
steps:
- name: Check out
uses: actions/checkout@v3
- name: Checkout source code
uses: actions/checkout@v4
with:
token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
fetch-depth: 0
- name: Set up Python
uses: actions/setup-python@v3
with:
python-version: '3.x'
uses: actions/setup-python@5
- name: Install dependencies
run: |
python -m pip install --upgrade pip
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/sync.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ on:

jobs:
sync-branches:
name: Sync dev and patch branches to latest commit
runs-on: ubuntu-latest
name: "Sync dev and patch branches to latest commit"
steps:
- name: Check out
uses: actions/checkout@v3
- name: Checkout source code
uses: actions/checkout@v4
with:
token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
fetch-depth: 0
Expand Down
15 changes: 6 additions & 9 deletions .github/workflows/main.yml → .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,31 @@ name: Tests
on:
push:
workflow_dispatch:
pull_request:
types: [opened, edited, reopened, synchronize]

jobs:
test_latest:
name: Run tests against ckantest latest
runs-on: ubuntu-latest

steps:
- name: Checkout source code
uses: actions/checkout@v3

uses: actions/checkout@v4
- name: Build images
run: docker compose build

- name: Run tests against latest
env:
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
run: docker compose run -e COVERALLS_REPO_TOKEN latest bash /opt/scripts/run-tests.sh -c ckanext.doi

test_next:
name: Run tests against ckantest next
runs-on: ubuntu-latest

steps:
- name: Checkout source code
uses: actions/checkout@v3

uses: actions/checkout@v4
- name: Build images
run: docker compose build

- name: Run tests against next
env:
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
run: docker compose run -e COVERALLS_REPO_TOKEN next bash /opt/scripts/run-tests.sh -c ckanext.doi
25 changes: 13 additions & 12 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,33 +1,34 @@
exclude: /(vendor|dist)/
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.3.0
rev: v5.0.0
hooks:
- id: check-merge-conflict
- id: detect-private-key
- id: end-of-file-fixer
- id: name-tests-test
args: ["--pytest-test-first"]
args: ['--pytest-test-first']
exclude: ^tests/helpers/
- id: trailing-whitespace
- repo: https://github.com/commitizen-tools/commitizen
rev: v2.37.0
rev: v3.30.0
hooks:
- id: commitizen
additional_dependencies: ["cz-nhm"]
- repo: https://github.com/psf/black
rev: 22.10.0
additional_dependencies: ['cz-nhm']
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.7.1
hooks:
- id: black
- id: ruff
args: [ '--fix', '--select', 'I', '--select', 'F401', '--fix-only' ]
- id: ruff-format
- repo: https://github.com/PyCQA/docformatter
rev: v1.5.0
rev: eb1df34
hooks:
- id: docformatter
# these can't be pulled directly from the config atm, not sure why
args: ["-i", "--wrap-summaries=88", "--wrap-descriptions=88",
"--pre-summary-newline", "--make-summary-multi-line"]
args: [ '-i', '--config', './pyproject.toml' ]
additional_dependencies: ['tomli']
- repo: https://github.com/pre-commit/mirrors-prettier
rev: v3.0.0-alpha.4
rev: v4.0.0-alpha.8
hooks:
- id: prettier
types_or: [ javascript, vue, less, sass, scss, css ]
Expand Down
19 changes: 11 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ This extension and [several others](https://github.com/search?q=topic:ckan+org:N

The current core team consists of:
- Josh ([@jrdh](https://github.com/jrdh)) - Technical Lead
- Ginger ([@alycejenni](https://github.com/alycejenni)) - Software Engineer
- Ginger ([@alycejenni](https://github.com/alycejenni)) - Senior Software Engineer


## Questions
Expand Down Expand Up @@ -81,7 +81,10 @@ The process is generally as follows:
3. Make your changes, and commit often; each commit should only contain one change. See below for specifics on how to word your commits.
4. Push your changes back to your fork.
5. [Open a pull request](https://docs.github.com/en/get-started/quickstart/contributing-to-projects#making-a-pull-request) in this repository, with the base branch set to **dev** and the compare branch set to your new branch. Provide a summary of your changes in the description.
6. If the automatic tests fail (these may take a while), please go back to your code and try to make them pass. You may have to update the tests themselves. You don't have to close the pull request while you're doing this; it'll update as you add further commits.
6. There are several automated checks that will run when you open the pull request. Try to make all of them pass. If you do not at least _attempt_ to make them pass, we will not merge your pull request.
1. Tests. Update them so that they pass, if necessary. New tests are always welcome in any pull request, but if you have added a new feature that has decreased the coverage, new tests are required.
2. Commit format validation. If you have not followed the conventional commits format for one or more of your commits, this will fail.
3. Code format validation. If you have not formatted your code correctly (using Ruff, docformatter, and/or Prettier), this will fail.
7. Wait for feedback from one of the core maintainers. If it's been a week or so and we haven't responded, we may not have seen it. You can find other places to contact us in [SUPPORT.md](./.github/SUPPORT.md).

### Commits
Expand Down Expand Up @@ -142,9 +145,9 @@ cz c

##### pre-commit

pre-commit is a tool that runs a variety of checks and modifications before a commit is made. You can check the [.pre-commit-config.yaml](./.pre-commit-config.yaml) file to see eaxtly what it's currently configured to do for this repository, but of particular note:
pre-commit is a tool that runs a variety of checks and modifications before a commit is made. You can check the [.pre-commit-config.yaml](./.pre-commit-config.yaml) file to see exactly what it's currently configured to do for this repository, but of particular note:

- reformats Python code with [Black](https://github.com/psf/black)
- reformats Python code with [Ruff](https://docs.astral.sh/ruff)
- reformats JavaScript and stylesheets with [Prettier](https://prettier.io)
- reformats docstrings with [docformatter](https://github.com/PyCQA/docformatter)
- checks your commit message is correcly formatted
Expand All @@ -161,15 +164,15 @@ pre-commit run

Don't forget to stage any modifications that it makes! Once it runs without failing, then you can make your commit.

Something to remember is that empty docstrings will cause conflicts between Black and docformatter and the checks will fail repeatedly - so don't leave your docstrings empty!
Something to remember is that empty docstrings will cause conflicts between Ruff and docformatter and the checks will fail repeatedly - so don't leave your docstrings empty!

### Code changes and style guide

We generally use external style guides and tools to help us maintain standardised code. Black and Prettier will be run with pre-commit.
We generally use external style guides and tools to help us maintain standardised code. Ruff and Prettier will be run with pre-commit.

#### Python

We follow the [Black style](https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html), with the notable exception that we use single quotes.
We use [Ruff](https://docs.astral.sh/ruff) to format our code, using defaults for everything except quote style (we use single quotes).

We also _mostly_ use [CKAN's style](http://docs.ckan.org/en/latest/contributing/python.html), with the following exceptions:
- prefer `f''` strings over `.format()`
Expand All @@ -178,7 +181,7 @@ We also _mostly_ use [CKAN's style](http://docs.ckan.org/en/latest/contributing/

#### JavaScript and stylesheets (CSS, LESS, etc)

We use [Prettier](https://prettier.io) to format these files.
We use [Prettier](https://prettier.io) to format these files. As with Ruff, we use defaults for everything except quote style (we use single quotes).

#### Accessibility

Expand Down
10 changes: 5 additions & 5 deletions ckanext/doi/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ def build_metadata_dict(self, pkg_dict, metadata_dict, errors):
the build_metadata_dict method from ckanext-doi.

:param pkg_dict: package dictionary
:param metadata_dict: the current metadata dict, created by the ckanext-doi extension and
any previous plugins implementing IDoi
:param errors: a dictionary of metadata keys and errors generated by previous plugins;
this method should remove any keys that it successfully processes and
overwrites
:param metadata_dict: the current metadata dict, created by the ckanext-doi
extension and any previous plugins implementing IDoi
:param errors: a dictionary of metadata keys and errors generated by previous
plugins; this method should remove any keys that it successfully processes
and overwrites
:returns: metadata_dict, errors
"""
return metadata_dict, errors
Expand Down
27 changes: 13 additions & 14 deletions ckanext/doi/lib/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,18 @@
# This file is part of ckanext-doi
# Created by the Natural History Museum in London, UK

import string

import logging
import random
import string
from datetime import datetime as dt

import xmltodict
from ckan.common import asbool
from ckan.plugins import toolkit
from ckanext.doi.model.crud import DOIQuery
from datacite import DataCiteMDSClient, schema42
from datacite.errors import DataCiteError, DataCiteNotFoundError
from datetime import datetime as dt

from ckanext.doi.lib.helpers import doi_test_mode
from ckanext.doi.model.crud import DOIQuery

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -48,7 +47,7 @@ def test_mode(self):
Whether to run in test mode.

Defaults to true.
:return: test mode enabled as boolean (true=enabled)
:returns: test mode enabled as boolean (true=enabled)
"""
if self._test_mode is None:
self._test_mode = doi_test_mode()
Expand All @@ -59,7 +58,7 @@ def get_prefix(cls):
"""
Get the prefix to use for DOIs.

:return: config prefix setting
:returns: config prefix setting
"""
prefix = toolkit.config.get('ckanext.doi.prefix')
if prefix is None:
Expand All @@ -75,10 +74,10 @@ def generate_doi(self):
"""
Generate a new DOI which isn't currently in use.

The database is checked for previous
usage, as is Datacite itself. Use whatever value is retuned from this function quickly to
avoid double use as this function uses no locking.
:return: the full, unique DOI
The database is checked for previous usage, as is Datacite itself. Use whatever
value is retuned from this function quickly to avoid double use as this function
uses no locking.
:returns: the full, unique DOI
"""
# the list of valid characters is larger than just lowercase and the digits but we don't
# need that many options and URLs with just alphanumeric characters in them are nicer. We
Expand Down Expand Up @@ -134,7 +133,7 @@ def set_metadata(self, doi, xml_dict):

:param doi: the DOI to update the metadata for
:param xml_dict: the metadata as an xml dict (generated from build_xml_dict)
:return:
:returns:
"""
xml_dict['identifiers'] = [{'identifierType': 'DOI', 'identifier': doi}]

Expand All @@ -150,7 +149,7 @@ def get_metadata(self, doi):
Retrieve metadata for a given DOI on datacite.

:param doi: the DOI for which to retrieve the stored metadata
:return:
:returns:
"""
try:
metadata = self.client.metadata_get(doi)
Expand All @@ -164,7 +163,7 @@ def check_for_update(self, doi, xml_dict):

:param doi: the DOI of the package
:param xml_dict: the xml_dict generated by build_xml_dict
:return: True if the two are the same, False if not
:returns: True if the two are the same, False if not
"""
posted_xml = self.get_metadata(doi)
if posted_xml is None or posted_xml.strip() == '':
Expand Down
4 changes: 2 additions & 2 deletions ckanext/doi/lib/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def date_or_none(date_object_or_string):
Try and convert the given object into a datetime; if not possible, return None.

:param date_object_or_string: a datetime or date string
:return: datetime or None
:returns: datetime or None
"""
if isinstance(date_object_or_string, datetime):
return date_object_or_string
Expand All @@ -63,6 +63,6 @@ def doi_test_mode():
"""
Determines whether we're running in test mode.

:return: bool
:returns: bool
"""
return toolkit.asbool(get_setting('ckanext.doi.test_mode', default=get_debug()))
2 changes: 1 addition & 1 deletion ckanext/doi/lib/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def build_xml_dict(metadata_dict):
generate the xml.

:param metadata_dict: a dict of metadata generated from build_metadata_dict
:return: dict that can be passed directly to datacite.schema42.tostring()
:returns: dict that can be passed directly to datacite.schema42.tostring()
"""

# required fields first (DOI will be added later)
Expand Down
17 changes: 10 additions & 7 deletions ckanext/doi/lib/xml_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,19 @@ def create_contributor(
Create a dictionary representation of a contributing entity (either a person or an
organisation) for use in an xml_dict.

:param full_name: the full name of the creator, in the format "FamilyName, GivenName";
can be omitted if family_name and given_name are provided
:param family_name: family name of the creator; will be ignored if given_name is None
:param full_name: the full name of the creator, in the format "FamilyName,
GivenName"; can be omitted if family_name and given_name are provided
:param family_name: family name of the creator; will be ignored if given_name is
None
:param given_name: given name(s) or initials of the creator; will be ignored if
family_name is None
family_name is None
:param is_org: sets name type to Organizational if true
:param contributor_type: the contributor type to set
:param affiliations: affiliations of the contributor, either a string or list of strings
:param identifiers: a list of dicts with "identifier", "scheme", and (optionally) "scheme_uri"
:return: a dict
:param affiliations: affiliations of the contributor, either a string or list of
strings
:param identifiers: a list of dicts with "identifier", "scheme", and (optionally)
"scheme_uri"
:returns: a dict
"""
if is_org and full_name is None:
raise ValueError('Creator name must be supplied as full_name="Org Name"')
Expand Down
Loading
Loading