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

Updating cl.yml redirects #890

Merged
merged 4 commits into from
Jan 9, 2023
Merged

Conversation

shawntanzk
Copy link
Contributor

@anitacaron could you help me make sure these redirects are correctly changed please? :) thanks

@anitacaron could you help me make sure these redirects are correctly changed please? :) thanks
@shawntanzk
Copy link
Contributor Author

hold off merging till we make sure cl-plus can be uploaded to a release

@shawntanzk shawntanzk marked this pull request as draft December 15, 2022 10:27
Copy link
Contributor

@matentzn matentzn left a comment

Choose a reason for hiding this comment

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

I will let @jamesaoverton comment on this but all these "exact" redirects seem to be redundant to the product list to me..

@shawntanzk shawntanzk marked this pull request as ready for review December 15, 2022 13:10
@shawntanzk
Copy link
Contributor Author

cl-plus can be uploaded to release, and we are on the way to making the release tomorrow (hopefully), probably best not to merge until that is done, but otherwise this is ready for review

config/cl.yml Outdated
- cl.obo: https://github.com/obophenotype/obophenotype/cell-ontology/releases/latest/download/cl.obo
- cl-basic.owl: https://github.com/obophenotype/obophenotype/cell-ontology/releases/latest/download/cl-basic.owl
- cl-basic.obo: https://github.com/obophenotype/obophenotype/cell-ontology/releases/latest/download/cl-basic.obo
- cl-plus.owl: https://github.com/obophenotype/obophenotype/cell-ontology/releases/latest/download/cl-plus.owl
Copy link
Member

Choose a reason for hiding this comment

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

The products list should be limited to IDSPACE.FORMAT, so cl-basic.owl and cl-basic.obo should not be allowed. It looks like they've been in here for 7 years, since the initial migration, so I guess we'll keep that. But I don't want to add any more exceptions, so please remove cl-plus.owl and cl-plus.obo. They should have exact entries below.

Copy link
Member

@jamesaoverton jamesaoverton left a comment

Choose a reason for hiding this comment

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

Please remove cl-plus.* items from products. There's an extra obophenotype/ in most of the other changes that needs to be rmeoved.

@jamesaoverton
Copy link
Member

@matentzn No, the exact entries are not redundant. The products entries control the "top-level" space under http://purl.obolibrary.org/obo/ and are highly restricted, while the exact entries control the "project space" under http://purl.obolibrary.org/obo/cl/ and are nearly unrestricted.

@matentzn
Copy link
Contributor

@jamesaoverton I never knew that! Good to know :) Thanks for the clarification.

@shawntanzk
Copy link
Contributor Author

Think I fixed it, thanks @jamesaoverton & @matentzn

Copy link
Member

@jamesaoverton jamesaoverton left a comment

Choose a reason for hiding this comment

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

I checked some of these updated links and all I got was 404 errors. You should be able to paste the links into a browser and get a file. If they don't work in your browser, they won't work in the PURL server. Please make sure that they all resolve.

@shawntanzk
Copy link
Contributor Author

oh yeah, it needs this to be merged first: obophenotype/cell-ontology#1767
didn't know which order it should be in so was hoping to get approval and then merge them both together.

@jamesaoverton
Copy link
Member

My understanding of that PR is that you're currently storing the release OWL files in git, but you're going to start attaching them to GitHub releases instead. So there's a synchronization problem. I see two options:

First, if you're not worried about downtime, then coordinate with me or @matentzn to merge this right after CL #1767.

Second, to avoid downtime and be more cautions, do this in three steps:

  1. change the PURL config to point to the 2022-12-15 OWL files in git
  2. merge CL #1767 and attach OWL files to the release
  3. change the PURL config to point to the files attached to the GitHub release

@shawntanzk
Copy link
Contributor Author

Sorry forgot to reply here, I think a bit of downtime is not a huge deal, I'll wait for obophenotype/cell-ontology#1767 to be merged first, once that is done, I'll ping here and get this reviewed and merged :) thanks heaps

@shawntanzk shawntanzk requested review from jamesaoverton and matentzn and removed request for jamesaoverton January 9, 2023 13:57
@shawntanzk
Copy link
Contributor Author

@jamesaoverton I've changed CL release and the links should resolve now, would you mind helping me give a thumbs up if all is good? Thanks

@jamesaoverton jamesaoverton merged commit f2e2806 into OBOFoundry:master Jan 9, 2023
@shawntanzk
Copy link
Contributor Author

Thanks for doing this so fast @jamesaoverton! really appreciate it :)

@jamesaoverton
Copy link
Member

Yes, it looks good. I'll merge now, so the PURLs will be updated in < 20 minutes. You can make further changes if needed.

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