Skip to content

Correct URI scheme to http #197

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

Merged
merged 1 commit into from
Jul 31, 2025
Merged

Conversation

zepheiryan
Copy link
Contributor

@zepheiryan zepheiryan commented Jul 30, 2025

Correct this property scheme to match all others in this file.

Copy link

Jest Unit Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit f79e9dd. ± Comparison against base commit 42bd36e.

Copy link

@zepheiryan zepheiryan requested review from pkjacob and a team July 30, 2025 22:17
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

Yay for consistency, and I will approve this simple PR on that principle and the fact that I recognize the users involved here and I know that you know what you're doing, but:

  • Moving to http from https feels like a step backward. While this one https feels like the outlier, how do we know it's not the last Right Thing holding down the fort of authentication and data integrity? as noted below, this value looks like a URL but here it's just an identifier, so http vs https is insignificant.
  • Is there a related Jira that describes why this work is necessary, i.e. countering the point above? That'll also provide visibility in Jira (where version information is tracked) into which release contains this fix. While we sometimes accept documentation changes that are unattached to tickets, it is expected that every code-change PR is linked to a Jira.

@pkjacob
Copy link
Contributor

pkjacob commented Jul 31, 2025

Yay for consistency, and I will approve this simple PR on that principle and the fact that I recognize the users involved here and I know that you know what you're doing, but:

  • Moving to http from https feels like a step backward. While this one https feels like the outlier, how do we know it's not the last Right Thing holding down the fort of authentication and data integrity?
  • Is there a related Jira that describes why this work is necessary, i.e. countering the point above? That'll also provide visibility in Jira (where version information is tracked) into which release contains this fix. While we sometimes accept documentation changes that are unattached to tickets, it is expected that every code-change PR is linked to a Jira.

Hi @zepheiryan
These are RDF IRIs defined in the builde vocabulary - https://bibfra.me/
IRIs are usually http.

This is what chatgpt says -
image

@zburke
Copy link
Member

zburke commented Jul 31, 2025

Thanks, @pkjacob , "RDF IDIs are identifiers, not necessarily fetch URLs" is exactly the kind of context I was looking for. The code in the file being changed here offers some misdirection (the variable containing this value is BFLITE_URIS) and the PR itself lacks direction (there is no explanation of why this change is happening, or a reference to a ticket).

If your team wants feedback from @folio-org/fe-tl-reviewers, please make sure the PRs provide us with enough context to evaluate them :) I trust you to validate what ChatGPT is saying, but on my own I wouldn't know if that summary was legit or a hallucination.

@zepheiryan zepheiryan merged commit 0d7a00b into master Jul 31, 2025
15 checks passed
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