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

Add planning final status fields and codelist #1642

Merged
merged 8 commits into from
May 7, 2024

Conversation

duncandewhurst
Copy link
Contributor

@duncandewhurst duncandewhurst commented Oct 9, 2023

@duncandewhurst duncandewhurst marked this pull request as ready for review October 9, 2023 01:33
@duncandewhurst duncandewhurst requested a review from odscjen October 9, 2023 01:34
@duncandewhurst
Copy link
Contributor Author

The spell-check failure is addressed by #1641

Copy link
Contributor

@odscjen odscjen left a comment

Choose a reason for hiding this comment

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

My reading of #1582 and the associated CRM issue is that it's not just the planning final status date that's needed by also a tender.finalStatusDate

schema/codelists/planningFinalStatus.csv Outdated Show resolved Hide resolved
schema/release-schema.json Outdated Show resolved Hide resolved
@duncandewhurst
Copy link
Contributor Author

Ah, good spot. I shouldn't have marked #1582 as being closed by this PR. I figure it makes sense to do those other final status date fields when adding the new final status fields in #1160.

Alternatively, we could do all of the final status and final status date fields in this PR. @jpmckinney let me know what you prefer.

@jpmckinney
Copy link
Member

My comment in #1583 was to close it "After #1160 and #1582". #1583 is more like a new feature, whereas #1160 is a major change (#1582 is also more like a new feature). The major change should lead, before related features are implemented.

I prefer to close #1160 on its own. #1582 and #1583 could potentially be combined.

@duncandewhurst
Copy link
Contributor Author

duncandewhurst commented Oct 11, 2023

Sounds good! I'll pause work on this PR and prepare a PR for #1160 (PR: #1648)

@jpmckinney
Copy link
Member

@duncandewhurst This is now unblocked.

@duncandewhurst
Copy link
Contributor Author

I think this is ready for review, unless I missed something.

@jpmckinney jpmckinney force-pushed the 1583-planning-final-status branch from f9b3138 to f42dd58 Compare May 4, 2024 03:39
Copy link
Member

@jpmckinney jpmckinney left a comment

Choose a reason for hiding this comment

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

I aligned 'cancelled' with all other 'cancelled' in other codelists (basically, it's a decision by the buyer, always for the same potential reasons).

I also aligned 'unsuccesful' in terms of structure, and using a less specific example. A specific example, is e.g. a building application where residents might reject it.

@jpmckinney jpmckinney requested a review from odscjen May 4, 2024 03:39
@jpmckinney jpmckinney merged commit 6249189 into 1.2-dev May 7, 2024
10 checks passed
@jpmckinney jpmckinney deleted the 1583-planning-final-status branch May 7, 2024 13:46
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.

Planning final status codelist
3 participants