-
Notifications
You must be signed in to change notification settings - Fork 0
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
Story/cico 13 #11
base: develop
Are you sure you want to change the base?
Story/cico 13 #11
Conversation
…ommit, started working on fixing add_item method, updated requirements.txt, started working on PyPi structure
Created new PR to track all tickets related to CICO-6 |
…, updated .toml file, added some tests
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 don't think this action is doing anything? Just checking out and installing ruff?
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.
Also, the action should run the tests.
|
||
For retrieivng the Bearer access token required for endpoint method calls, please see the following OAuth2 [documentation] (https://diging.atlassian.net/wiki/spaces/OAC/pages/3533078792/Getting+OAuth2+Access+Token+in+Postman) for Citesphere | ||
|
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 the pre-commit hooks need to be installed too, don't they?
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.
It was my understanding that as long as the pre-commit yaml file is present, you don't need to directly install anything
pyproject.toml
Outdated
|
||
[project.urls] | ||
"Citesphere API" = "https://documenter.getpostman.com/view/19365454/UVeMJiyx" | ||
"Citesphere" = "https://diging-dev.asu.edu/citesphere-review/" |
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.
Let's take this out.
requirements.txt
Outdated
pre_commit==4.0.1 | ||
PyYAML==6.0.2 | ||
ruff==0.7.4 | ||
virtualenv==20.27.1 |
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 don't think this needs to be installed?
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.
And does it really need all these dependencies?
setup.py
Outdated
python_requires=">=3.9", | ||
project_urls={ | ||
"Citesphere API": "https://documenter.getpostman.com/view/19365454/UVeMJiyx", | ||
"Citesphere": "https://diging-dev.asu.edu/citesphere-review/", |
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.
same here
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.
pychache folder should not be pushed
…connector into story/CICO-13 divergent due to file deletion
Guidelines for Pull Requests
If you haven't yet read our code review guidelines, please do so, You can find them here.
Please confirm the following by adding an x for each item (turn
[ ]
into[x]
).Please provide a brief description of your ticket
(you can copy the ticket if it hasn't changed)
Update and clean up CitesphereConnector
Anything else the reviewer needs to know?