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.

Sign up for GitHub

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

Jump to bottom

Avoid code repetition between import_info and import_info_from_list #14

Open
jvfe opened this issue Apr 10, 2022 · 2 comments · May be fixed by #16
Open

Avoid code repetition between import_info and import_info_from_list #14

jvfe opened this issue Apr 10, 2022 · 2 comments · May be fixed by #16
Labels
refactor Changes to code without affecting functionality

Comments

@jvfe
Copy link
Collaborator

jvfe commented Apr 10, 2022

Since the end result between the modules is essentially the same, the functionality desired - to read from a list of ORCIDs - could be achieved by having a type guard on import_info.py, to see if the argument provided is a Path() that exists - thereby reading it and looping through them - or a string - in such case, only sending it to render_orcid_qs. I believe it would make the code simpler and cleaner.
jvfe added a commit to jvfe/pyorcidator that referenced this issue Apr 10, 2022
@jvfe
refactor: Unite orcid list & single orcid function
40126eb 
- Instead of repeating code between import info from list and import
  info, include a type guard on import_info to check if the argument
provided is a path or a just a string.

- Addresses lubianat#14
@jvfe
Copy link
Collaborator Author

jvfe commented Apr 10, 2022

40126eb shows how I'd go about implementing what I said above. If you want I can open a PR with it.

@lubianat
Copy link
Owner

Looks better, @jvfe! a PR would be great

@jvfe jvfe linked a pull request Apr 10, 2022 that will close this issue
Open
@jvfe jvfe added the refactor Changes to code without affecting functionality label Apr 11, 2022
jvfe added a commit to jvfe/pyorcidator that referenced this issue Apr 15, 2022
@jvfe
refactor: Unite orcid list & single orcid function
ffd5814 
- Instead of repeating code between import info from list and import
  info, include a type guard on import_info to check if the argument
provided is a path or a just a string.

- Addresses lubianat#14
jvfe added a commit to jvfe/pyorcidator that referenced this issue Apr 15, 2022
@jvfe
refactor: Keep 2 commands but reduce repetition
84142c8 
- Keeps both commands, but refactors repeated functionality into a
  separate function.
- Also improves the way the path to the ORCID file is read
- Also adds missing open_browser to import_list

- Closes lubianat#14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
refactor Changes to code without affecting functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid code repetition among import and import_list jvfe/pyorcidator
2 participants
@lubianat @jvfe