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

Define every url and viewUrl field as format uri #200

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

davidB
Copy link
Contributor

@davidB davidB commented Apr 7, 2024

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

@davidB davidB requested a review from a team as a code owner April 7, 2024 15:39
@afrittoli afrittoli added this to the v0.5 milestone Apr 9, 2024
@afrittoli
Copy link
Contributor

Thanks @davidB - we discussed this topic during the last CDEvents WG and we plan to include it in the next milestone v0.5. We'll try to have most of the breaking changes in one milestone (v0.5). Once the v0.4 release is out (this week), this PR will have to be rebased and the version of the affected events updated.

@afrittoli
Copy link
Contributor

@davidB - main is now open for v0.5 development - could you rebase your PR based on top of the updated main?
@xibz @obowersa this PR is related to #162 and cleanup/consistency work we plan for v0.5.

@xibz xibz added the breaking change Indicates when a PR or issue will have breaking changes label Apr 15, 2024
@davidB davidB force-pushed the fix_191_set_url_consistent branch from 0e9f1b7 to 410359a Compare October 5, 2024 14:31
@davidB
Copy link
Contributor Author

davidB commented Oct 5, 2024

@afrittoli The PR is rebased on main.

Copy link
Contributor

@xibz xibz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@xibz xibz left a comment

Choose a reason for hiding this comment

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

We should rename URL to URIs

@davidB davidB force-pushed the fix_191_set_url_consistent branch from a2c758d to cc5754a Compare November 19, 2024 13:54
@davidB
Copy link
Contributor Author

davidB commented Nov 19, 2024

We should rename URL to URIs

@xibz I renamed every field named url into uri. But kept viewUrl as is.

@davidB davidB requested a review from xibz November 19, 2024 13:57
@davidB
Copy link
Contributor Author

davidB commented Dec 4, 2024

@xibz / @afrittoli Can you merge the PR (I'm not allowed).

Copy link
Contributor

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thanks @davidB - the change looks good.
Since all event versions have been bumped already and are currently in -draft this looks fine, no further updates required.

@afrittoli afrittoli merged commit 3e2e806 into cdevents:main Dec 4, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Indicates when a PR or issue will have breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants