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

#222: Use ASnake to format the resource id #236

Merged
merged 3 commits into from
Sep 1, 2022
Merged

Conversation

ctgraham
Copy link
Contributor

@ctgraham ctgraham commented Sep 1, 2022

Resolves #222.

I felt a little torn as to whether this really deserved a helper method, but I didn't see any direct interaction with the ASnake client in routines.py other than the initial object fetch.

@ctgraham
Copy link
Contributor Author

ctgraham commented Sep 1, 2022

Is this a python linter asserting that the import functions should be in alphabetical order?

diff --git a/process_request/helpers.py b/process_request/helpers.py
index b639480..15cd9bb 100644
--- a/process_request/helpers.py
+++ b/process_request/helpers.py
@@ -3,7 +3,8 @@ import re

 import inflect
 import shortuuid
-from asnake.utils import get_date_display, get_note_text, text_in_note, format_resource_id
+from asnake.utils import (format_resource_id, get_date_display, get_note_text,
+                          text_in_note)
 from django.conf import settings
 from ordered_set import OrderedSet

 

@@ -447,4 +448,3 @@ def get_formatted_resource_id(resource, client):
     Concatenates the resource id parts using the separator from the config
     """
     return format_resource_id(resource, client, settings.RESOURCE_ID_SEPARATOR)
-
diff --git a/process_request/routines.py b/process_request/routines.py
index 4fbd77b..e9911e2 100644
--- a/process_request/routines.py
+++ b/process_request/routines.py
@@ -5,10 +5,11 @@ from asnake.aspace import ASpace
 from django.conf import settings
 from django.core.mail import send_mail
 
-from .helpers import (get_container_indicators, get_dates, get_parent_title,
+from .helpers import (get_container_indicators, get_dates,
+                      get_formatted_resource_id, get_parent_title,
                       get_preferred_format, get_resource_creators,
                       get_restricted_in_container, get_rights_info, get_size,
-                      get_url, list_chunks, get_formatted_resource_id)
+                      get_url, list_chunks)

And that process_request/helpers.py should not have a trailing CRLF?

@helrond
Copy link
Member

helrond commented Sep 1, 2022

Yeah I feel torn on whether a helper is fully necessary as well, but I'm fine with it as-is.

Linting - I too have an incredibly hard time reading those diffs. What I would suggest: install pre-commit locally and then do pre-commit install at the root of the repository. This will install git hooks which run the linters each time you do git-commit and will also autofix most of the issues. If you don't want to have those git hooks installed you can (I think) just run pre-commit run --all-files (i.e. the same command that's in the Travis config) and it should fix things for you as well.

@helrond helrond merged commit a5bbc13 into development Sep 1, 2022
@helrond helrond deleted the issue-222 branch September 1, 2022 22:01
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