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

Feature/uiad v2 with dedicated description #678

Merged
merged 8 commits into from
Jan 22, 2025

Conversation

HelgeCD
Copy link
Contributor

@HelgeCD HelgeCD commented Dec 12, 2024

In the v1 version we did not have a dedicated field for the description in the database. Instead another field was used that is displayed in some scenarios (e.g. smart links, the default application title) in the FLP.

with the v2 version we now have a dedicated field for the description on the database and have therefore added a targetMappingTitle which is the old field which we misused as description.

@schneidermic0
Copy link
Contributor

Technically, you have just added a new field. That would be even a compatible change and could have been done with format version 1.

However, I understand you misused a v1 field for the description. Therefore, you need to increase the format version to distinguish both versions. Do I have the right understanding?

@HelgeCD
Copy link
Contributor Author

HelgeCD commented Jan 13, 2025

Technically, you have just added a new field. That would be even a compatible change and could have been done with format version 1.

However, I understand you misused a v1 field for the description. Therefore, you need to increase the format version to distinguish both versions. Do I have the right understanding?

yes, it is really to be able to differentiate the 2 versions on semantic level, so that we do not save the target mapping a description or the other way around.

Copy link
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

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

In general the new version looks good to me. I would suggest to keep the old format version in the repository and adapt the readme file accordingly.

I think I can also label it as ready for UX review

file-formats/uiad/README.md Show resolved Hide resolved
@schneidermic0 schneidermic0 added the ux-review ready AFF is ready for UX review label Jan 14, 2025
Copy link
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

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

Looks good to me, now...

@wurzka wurzka requested a review from a team January 22, 2025 13:07
Copy link
Contributor

@wurzka wurzka left a comment

Choose a reason for hiding this comment

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

From ux-perspective, looks good to me.

@wurzka wurzka merged commit 0d3ec0c into SAP:main Jan 22, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ux-review ready AFF is ready for UX review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants