Skip to content

Conversation

@ehanson8
Copy link
Contributor

@ehanson8 ehanson8 commented May 1, 2019

What does this PR do?

After testing some scripts against the new DSpace instance to confirm that I could authenticate with my account, I discovered an error in the authentication code block I was using across all scripts. This PR fixes that code block across the repo. I still want these to be self-contained procedural scripts for easier reuse by others who may be newer to coding. However, I will likely redesign my approach to this work as we start building up more API-based workflows around DSpace down the line.

Includes new or updated dependencies?

NO

Copy link
Contributor

@hakbailey hakbailey left a comment

Choose a reason for hiding this comment

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

It looks like you're repeating this authentication code block in most of the files in this repo. Since you've found an error in it and have to edit every file anyway, I would strongly suggest moving the authentication process into a function that gets called by each script, rather than continuing to duplicate the code everywhere. It will be much easier to maintain and test.

@ehanson8
Copy link
Contributor Author

ehanson8 commented May 2, 2019

After discussion with Helen, I will merge this PR so that the scripts are properly functional as standalone entities and issue it as a release. Afterward, I will begin breaking out repeated code blocks as functions for easier editing and testing

@ehanson8 ehanson8 merged commit 7f2c25f into master May 2, 2019
@ehanson8 ehanson8 deleted the authentication-upgrade branch May 2, 2019 17:52
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