camaraproject / PopulationDensityData

Repository to describe, develop, document and test the Population Density Data API family
Apache License 2.0
3 stars 5 forks source link

Prepare release r1.2 #45

Closed jgarciahospital closed 1 month ago

jgarciahospital commented 2 months ago

What type of PR is this?

Add one of the following kinds:

What this PR does / why we need it:

Prepare the release 1.2 with the required changes.

Which issue(s) this PR fixes:

Fixes #35 Related to #44

Special notes for reviewers:

Pending to include #44 before merging this PR

bigludo7 commented 1 month ago

@jgarciahospital - to be sure we're aligned. I've added easy comments to fix for #44 & #47 - we solve them & merge them first and then we close this one - work for you?

jgarciahospital commented 1 month ago

@jgarciahospital - to be sure we're aligned. I've added easy comments to fix for #44 & #47 - we solve them & merge them first and then we close this one - work for you?

Thanks Ludovic for the review, comments applied in #44 and #47 and pending to be merged before close release PR. Please @sachinvodafone for your validation

tanjadegroot commented 1 month ago

I spotted a bug in the version used in the test file- see comment on #44

jgarciahospital commented 1 month ago

I spotted a bug in the version used in the test file- see comment on #44

Thanks @tanjadegroot , solved.

For information, API readiness checklist filename will be maintained as fixed in #44 after merging this PR

hdamker commented 1 month ago

@jgarciahospital @maheshc01 @sachinvodafone I approved the changes to remove the block from requested changes (they are addressed - thanks!). For me everything here looks good, but #44 and #47 need still to be approved and merged first. Would be good if you can do that asap so that we get this release still into the Meta-Release (we need to prepare the communication material the next days ... as M5 is approaching).

jgarciahospital commented 1 month ago

Great - LGTM.

The one point I found is not really relevant, but just confused me, as I know that the latest change was in ICM v0.2.0-rc.2 (which you have correctly applied already in r1.1)

Included for clarity, thanks @hdamker

tanjadegroot commented 1 month ago

@sachinvodafone @gregory1g @maheshc01 @rartych please review this PR, create the API release and update the API release tracker - we want to close the M4 today please :-)

hdamker commented 1 month ago

@jgarciahospital if the lack of an code owner review is blocking you, let me know and I help to merge. I see that @sachinvodafone has multiple times approved previously.

jgarciahospital commented 1 month ago

@jgarciahospital if the lack of an code owner review is blocking you, let me know and I help to merge. I see that @sachinvodafone has multiple times approved previously.

Conscious that API review was made during last meeting, and that we have at least one other company's approval, we can wait until tomorrow for a code owner approval or move directly if @hdamker you want to close today the milestone.

tanjadegroot commented 1 month ago

Approval on this PR from Release Management: LGTM

hdamker commented 1 month ago

Conscious that API review was made during last meeting, and that we have at least one other company's approval, we can wait until tomorrow for a code owner approval or move directly if @hdamker you want to close today the milestone.

We want to close today 👍

With the additional approval from @tanjadegroot I suppose we are good to go, will merge. @jgarciahospital may you take the creation of the release and the update of the release tracker?