Skip to content

Release r1.1 with 0.1.0-rc.1#15

Merged
lihaojie-CMCC merged 76 commits intomainfrom
lihaojie-CMCC-patch-1
Jul 24, 2025
Merged

Release r1.1 with 0.1.0-rc.1#15
lihaojie-CMCC merged 76 commits intomainfrom
lihaojie-CMCC-patch-1

Conversation

@lihaojie-CMCC
Copy link
Contributor

What type of PR is this?

Add one of the following kinds:

  • subproject management

What this PR does / why we need it:

Prepare the release r1.1:

  • Update CHANGELOG.md
  • Add API readiness checklist
  • Update API yaml files

Which issue(s) this PR fixes:

Fixes #12

Special notes for reviewers:

NOTE: PR #13 was accidentally closed. This PR serves as a follow-up to that work. For full context on previous progress/discussions, please refer to the history in #13 .

@lihaojie-CMCC lihaojie-CMCC requested a review from a team as a code owner July 1, 2025 04:04
@hdamker
Copy link
Contributor

hdamker commented Jul 1, 2025

NOTE: PR #13 was accidentally closed. This PR serves as a follow-up to that work. For full context on previous progress/discussions, please refer to the history in #13 .

BTW: you could have just reopened the PR, but it's also fine so. Please complete the release PR, until then I have converted it to "draft", as it is not yet ready for review. Use the "Ready for review" button when all needed changes are included.

README.md Outdated
# ModelAsAService
API Repository to describe, develop, document, and test the service APIs of the [ModelAsAService](https://lf-camaraproject.atlassian.net/wiki/x/AgDe) family
!! Replace the link with the link to the home page within the wiki and delete this task
API Repository to describe, develop, document, and test the service APIs of the [ModelAsAService](https://lf-camaraproject.atlassian.net/wiki/spaces/CAM/pages/52658427/ModelAsAService) family
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rebase your PR with current main branch, and keep the first 14 line which are currently in main branch unchanged (the link there is already correct, and we use the shorter "share" links),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

We've rebased the PR onto the latest main branch and confirmed the first 14 lines remain unchanged (including the correct short "share" link).

Regarding the README:
We've updated the meeting schedule and links in this PR. Could these changes be incorporated?

Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding the README:
We've updated the meeting schedule and links in this PR. Could these changes be incorporated?

Sure you can ... but in this case you need to resolve a merge conflict as there was also a change within the main branch.

Keep the first 14 line which are currently in main branch unchanged.
Update the meeting link.
Update link to Commonalities and ICM r3.2
Update link to Commonalities and ICM r3.2
Update link to Commonalities and ICM r3.2
@lihaojie-CMCC
Copy link
Contributor Author

We have gone through the Review actions list for ModelAsAService r1.1 (Fall25 M3, new APIs) release review item by item, checked and modified the relevant files as required.
We believe the current version is now ready for review.
Everyone is welcome to review and provide comments. Thanks again for all the work.

@lihaojie-CMCC lihaojie-CMCC marked this pull request as ready for review July 4, 2025 10:29
@Kevsy
Copy link
Contributor

Kevsy commented Jul 21, 2025

Thanks @lihaojie-CMCC :

API User Story is optional, but please change the API Readiness Checklist to say 'N' (I think it says 'tbc' at the moment)

Yes the bot review is complete 😁 , but there is also my manual review. I have included knowledge-base.yaml and qa-assistant-manage.yaml below in this comment, and I will complete the other files later

knowledge-base.yaml

  1. Formatting issue: please leave a carriage return/blank line after the 'Security and Authentication' section header. Otherwise the whole section is in bold. See here:

## Security and Authentication

  1. The scope documents.upload does not follow CAMARA Scope naming. Update: Please change to knowledge-base:documents:update

  2. Please add some text to the API documentation to explain what the API does and why it would be used.

  3. All instances of knowledgeBaseID should be changed to knowledgeBaseId

qa-assistant-manage.yaml

  1. Formatting issue: please leave a carriage return/blank line after the 'Security and Authentication' section header. Otherwise the whole section is in bold

  2. The scopes need to follow the CAMARA Scope naming which specifies:

api-name:[resource:]action

So e.g. for the sessions resource, the scope should be: qa-assistant-manage:sessions:create and not assistant-manage:create

  1. Once you have changed these scopes, please update the 'Security and Authentication' section in the documentation with the correct scope names.

@Kevsy
Copy link
Contributor

Kevsy commented Jul 21, 2025

A few more manual review comments:

qa-assistant-service.yaml

  1. info.description says "API for querying answers from a QA assistant using a specified knowledge base and model configuration." -> Please can you say "Question and Answer (Q&A) assistant" because otherwise the reader may think 'QA' relates to 'Quality Assurance" (or 'Qualitative Analysis', etc.)

  2. Formatting issue: please leave a carriage return/blank line after the 'Security and Authentication' section header. Otherwise the whole section is in bold.

  3. The scope assistant:query does not follow CAMARA Scope naming. It should be: qa-assistant-service:read - please can you update in the security.openId object, and in the info.description?

@Kevsy
Copy link
Contributor

Kevsy commented Jul 23, 2025

@lihaojie-CMCC Please note I have updated the comment above with the correct scope syntax to apply in knowledge-base.yaml:

"2. The scope documents.upload does not follow CAMARA Scope naming. Update: Please change to knowledge-base:documents:update"

@lihaojie-CMCC
Copy link
Contributor Author

@Kevsy Thanks again for your thorough review! We've implemented all the suggested changes:

  • Formatting
  • Scope Naming
  • Enriched description content for clarity
  • Corrected knowledgeBaseID → knowledgeBaseId
  • Clarified Q&A assistant in descriptions

We've also verified and updated the Test definitions to reflect these modifications.

Your expertise really elevated the quality of this implementation – much appreciated!

@Kevsy
Copy link
Contributor

Kevsy commented Jul 23, 2025

Thanks @lihaojie-CMCC - nearly there, just a few suggestions added above 👍

lihaojie-CMCC and others added 6 commits July 23, 2025 18:21
Co-authored-by: Kevin Smith <Kevsy@users.noreply.github.com>
Co-authored-by: Kevin Smith <Kevsy@users.noreply.github.com>
Co-authored-by: Kevin Smith <Kevsy@users.noreply.github.com>
Co-authored-by: Kevin Smith <Kevsy@users.noreply.github.com>
Co-authored-by: Kevin Smith <Kevsy@users.noreply.github.com>
Co-authored-by: Kevin Smith <Kevsy@users.noreply.github.com>
servers:
- url: '{apiRoot}/qa-assistant-manage/vwip'
servers:
- url: {apiRoot}/qa-assistant-manage/v0.1rc1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- url: {apiRoot}/qa-assistant-manage/v0.1rc1
- url: {apiRoot}/qa-assistant-manage/v0.1rc1

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry @lihaojie-CMCC I made a whitespace mistake in my initial suggestion - the suggestion above should fix it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries at all, @Kevsy – happens to the best of us! 😄 I originally thought it was another issue, but the whitespace fix is now sorted. Thanks for catching that!

Copy link
Contributor

@Kevsy Kevsy left a comment

Choose a reason for hiding this comment

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

Release PR Approved on behalf of Release Management

Next steps for the ModelAsAService codeowners:

  • PR merged (by API repository codeowner)
  • Release created within GitHub (by API repository codeowner)*
  • Release Tracker updated (with creation date of the release and the release tag link)

*Note please follow these instructions to make the pre-release

@Kevsy
Copy link
Contributor

Kevsy commented Jul 24, 2025

HI @lihaojie-CMCC , please see the required actions above: they are required to achieve M3 for Fall 25. They must be done by the repository codeowners (release management are not able to do them). Thanks!

@lihaojie-CMCC lihaojie-CMCC merged commit c81416b into main Jul 24, 2025
1 check passed
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.

Release PR for Fall25 M3 milestone

3 participants