ehn-dcc-development / eu-dcc-schema

Schema for the ehn DCC payload
Apache License 2.0
165 stars 59 forks source link

Patch/rc 131 dep vsets #120

Closed gabywh closed 2 years ago

gabywh commented 3 years ago

closes: #119 resolves: https://github.com/ehn-dcc-development/ehn-dcc-valuesets/issues/5

Removes valuesets from this repo. This will become the release/1.3.1 branch.

I updated the DCC JSON Schema version to 1.3.1 (and corresponding README.md) although this is debatable, which is why I did it in a separate commit.

In removing the valuesets from this repo (https://github.com/ehn-dcc-development/ehn-dcc-schema), there is absolutely NO change to the DCC JSON schema. However, we need a change in this DCC Schema repo to finally remove the valuesets from this repo (where they should never have been in the first place!). The default view for this repo is "latest release". Thus I will generate a new branch release/1.3.1 where the valueset files are no longer present and which can become the default view.

However, the JSON Schema version could stay (technically) at 1.3.0 since there is no change to the JSON Schema per se - but then we would have JSON Schema v1.3.0 for both release/1.3.0 and release/1.3.1 branches. So I updated the JSON Schema version to 1.3.1 to match the release branches. Thus also why I did this as a separate commit since you could argue that the JSON Schema version should remain unchanged.

gabywh commented 3 years ago

@ryanbnl Could you update the tests? We have a choice:

  1. move the valuesets to the test folder (shift one level down), or
  2. (better) refer to the "real" valuesets @ https://github.com/ehn-dcc-development/ehn-dcc-valuesets

Obvs (2) is better since we then have a SPOT for the valuesets, but I'll leave the final decision to you.

gabywh commented 3 years ago

@ryanbnl Could you update the tests? We have a choice:

Didn't happen after a week, so did it myself.

See latest commit for Makefile change to ensure valuesets from remote repo ehn-dcc-valuesets.git are available for tests

gabywh commented 3 years ago

some "scribble notes" off the top of my head:

See the Prerequisites section in the README.md, but here sth of a summary for a debian/ubuntu etc family Linux distro you would probably want (if you don't already have them):

/sudo apt install build-essential/

/sudo apt install nodejs (which is almost sickeningly massive)/

from that you can run at a cmd prompt:

/make install-ajv (if you don't have ajv already)/

and then e.g.

/make value_sets_current_latest/

which should get them locally for you, and

/make validate-examples/

will ensure that the examples run with the valuesets etc

This gives you at least a functional check (which the github checks already do, BTW) and some confidence in the changes even if you don't feel up to analyzing the Makefile changes themselves.

HTH,

Gaby

Op 02-08-2021 om 14:51 schreef Hynek:

@.**** approved this pull request.

I agree with the way how @gabywh https://github.com/gabywh resolved the problem with dual VS location. However, I was not able to verify chnges of the makefile (sorry, not my domain).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ehn-dcc-development/ehn-dcc-schema/pull/120#pullrequestreview-720166129, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASHCPHMCKB3BZ4LHDYKG64LT22IF3ANCNFSM5A3BTIOA.

andrewkay18981 commented 2 years ago

Would be good to get this merged in if possible. The outdated value sets in this repo have caused some confusion amongst colleagues. Makes sense to remove them from here to avoid any further confusion.

Thanks

gabywh commented 2 years ago

The value sets here should have been removed a long time ago. I already have a pr for 1.3.1 which I did last July (!) but since end July l am no longer working on this. Gaby.

Op do 24 feb. 2022 12:55 schreef andrewkay18981 @.***>:

Would be good to get this merged in if possible. The outdated value sets in this repo have caused some confusion amongst colleagues. Makes sense to remove them from here to avoid any further confusion.

Thanks

— Reply to this email directly, view it on GitHub https://github.com/ehn-dcc-development/ehn-dcc-schema/pull/120#issuecomment-1049784422, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASHCPHKZPHN3RBPAKE6QHQLU4YMEPANCNFSM5A3BTIOA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

kruzikh commented 2 years ago

@dslmeinte could you approve this PR and finalize it?

kruzikh commented 2 years ago

@dslmeinte could you approve this PR and finalize it?

ryanbnl commented 2 years ago

@kruzikh we've released a 1.3.1 in the meantime, so we need to rebase this and bump to 1.3.2. Will get on it, and clean up the other open PRs whilst I'm at it.

ryanbnl commented 2 years ago

@kruzikh we've released a 1.3.1 in the meantime, so we need to rebase this and bump to 1.3.2. Will get on it, and clean up the other open PRs whilst I'm at it.

ryanbnl commented 2 years ago

@kruzikh we've released a 1.3.1 in the meantime, so we need to rebase this and bump to 1.3.2. Will get on it, and clean up the other open PRs whilst I'm at it.

gabywh commented 2 years ago

NOTE base branch should be MAIN

The policy is upstream first https://docs.gitlab.com/ee/topics/gitlab_flow.htmlhttps://docs.gitlab.com/ee/topics/gitlab_flow.html and e.g. https://www.chromium.org/chromium-os/chromiumos-design-docs/upstream-first/ :

Merging into main and then cherry-picking into release is called an “upstream first” policy, which is also practiced by [Google](https://www.chromium.org/chromium-os/chromiumos-design-docs/upstream-first) and [Red Hat](https://www.redhat.com/en/blog/a-community-for-using-openstack-with-red-hat-rdo).
  1. The changes for the now released 1.3.1 should all already be on main after I had to fix the branches
  2. You should keep all changes on main and then create a branch from main when you are ready to make a new release