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 the missing ContentModel annotations #1952

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

fritzmg
Copy link
Contributor

@fritzmg fritzmg commented Jul 21, 2020

Q A
Fixed issues -
Docs PR or issue -

I recently noticed that many of the changes regarding the media player, youtube and vimeo content element weren't updated in the ContentModel's annotations.

Btw. wouldn't it be feasible to write an automatic check whether a newly added or changed property is present the respective model's annotations?

@fritzmg fritzmg added the bug label Jul 21, 2020
@fritzmg fritzmg added this to the 4.9 milestone Jul 21, 2020
@fritzmg fritzmg requested a review from a team July 21, 2020 11:44
@fritzmg fritzmg self-assigned this Jul 21, 2020
@m-vo
Copy link
Member

m-vo commented Jul 21, 2020

Btw. wouldn't it be feasible to write an automatic check whether a newly added or changed property is present the respective model's annotations?

Well, we could add a test/sniffer that uses the AnnotationReader to read the model files, find out the respective dca definition, look it up and compare it. But not sure that's worth the effort?

@leofeyer leofeyer merged commit f4a326d into contao:4.9 Jul 21, 2020
@leofeyer
Copy link
Member

Thank you @fritzmg.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jul 21, 2020

Well, we could add a test/sniffer that uses the AnnotationReader to read the model files, find out the respective dca definition, look it up and compare it. But not sure that's worth the effort?

Something like that, yeah. But probably not worth the effort.

@fritzmg fritzmg deleted the fix/content-model-props branch July 21, 2020 12:14
@leofeyer leofeyer changed the title Fix missing ContentModel annotations Add the missing ContentModel annotations Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants