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

Allow https build.fhir.org links for ciUrl #601

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

vadi2
Copy link
Contributor

@vadi2 vadi2 commented Sep 13, 2022

Allow http as well as https build.fhir.org links for ciUrl attribute.

This reduces the number of false positives - before & after.

@vadi2 vadi2 marked this pull request as draft September 13, 2022 14:57
@vadi2 vadi2 marked this pull request as ready for review September 13, 2022 15:02
Copy link
Collaborator

@lmckenzi lmckenzi left a comment

Choose a reason for hiding this comment

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

Why make this change? This will fail all of the existing files that use http:, which is what we're standardizing on. (My understanding is that with HTTP3, https goes away because everything is implicitly https.)

@vadi2
Copy link
Contributor Author

vadi2 commented Sep 13, 2022

It will not fail existing files that use http:, it allows for both. See the sample run at https://github.com/vadi2/JIRA-Spec-Artifacts/actions/runs/3046149892/jobs/4908596565 which demonstrates that both http and https work.

Copy link
Collaborator

@lmckenzi lmckenzi left a comment

Choose a reason for hiding this comment

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

I think we want to require http: everywhere. There's no reason for some IGs to use https and some to use http

@vadi2
Copy link
Contributor Author

vadi2 commented Sep 14, 2022

I can't say I agree with that reasoning - as of today https: is the gold standard, and many IG's are using such links already.

@lmckenzi
Copy link
Collaborator

They shouldn't be able to use such links - the validation rules here should prevent that... Where are you seeing them?

@vadi2
Copy link
Contributor Author

vadi2 commented Sep 14, 2022

Quite a few in the repo...

image

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.

None yet

2 participants