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.

Sign up for GitHub

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

Jump to bottom

remove autocast feature #1501

Merged
merged 19 commits into from
Oct 11, 2023
Merged

remove autocast feature #1501

merged 19 commits into from
Oct 11, 2023

Conversation

AlvaroVega
Copy link
Member

@AlvaroVega AlvaroVega commented Oct 5, 2023 •
edited
Loading

fix for #1498

@AlvaroVega AlvaroVega mentioned this pull request Oct 5, 2023
Closed
@AlvaroVega AlvaroVega marked this pull request as ready for review October 5, 2023 15:20
@mapedraza
Copy link
Collaborator

mapedraza commented Oct 5, 2023 •
edited
Loading

@jason-fox be aware of this change. There were weird situations casting strings as objects. This behaviour is not valid and is going to be removed in this PR.

@jason-fox
Copy link
Contributor

jason-fox commented Oct 6, 2023 •
edited
Loading

Its good to see these necessary structural changes occurring, but much like the removal of bidirectional attributes, this removal is again a breaking change - any chance you could group all these breaking changes together and then update the major version number so it is more obvious what is happening.

There is no need to go 4.x, 5.x, 6.x, 7.x for each little one, just moving to 4.x once the big stuff has been done

@mapedraza
Copy link
Collaborator

@jason-fox noted your feedback. Good to see you agree with those changes

mapedraza
mapedraza approved these changes Oct 6, 2023
View reviewed changes
Copy link
Collaborator

@mapedraza mapedraza left a comment

Choose a reason for hiding this comment

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

LGTM

test/unit/ngsi-ld/examples/contextRequests/updateContextAliasPlugin8.json Show resolved Hide resolved
@AlvaroVega
Copy link
Member Author

AlvaroVega commented Oct 9, 2023 •
edited
Loading

autocast feature was disabled globally. IMHO this PR is not a breaking change.

@fgalan
Copy link
Member

fgalan commented Oct 9, 2023

The following refereces to "autocast" should be also removed in this branch:

imagen

@mapedraza mapedraza mentioned this pull request Oct 9, 2023
Merged
fgalan
fgalan approved these changes Oct 9, 2023
View reviewed changes
Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

Athough we should wait until de-autocast PR in IOTA-UL (and maybe IOTA-JSON?) before merging, to merge all them at once.

@AlvaroVega AlvaroVega changed the title Task/remove autocast2 [WIP] remove autocast feature Oct 10, 2023
@fgalan fgalan mentioned this pull request Oct 11, 2023
Merged
@fgalan
Copy link
Member

fgalan commented Oct 11, 2023

Not for version 3.5.0

@AlvaroVega AlvaroVega mentioned this pull request Oct 11, 2023
Merged
@AlvaroVega AlvaroVega changed the title [WIP] remove autocast feature remove autocast feature Oct 11, 2023
fgalan
fgalan approved these changes Oct 11, 2023
View reviewed changes
Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

@fgalan fgalan merged commit 87f9ee7 into master Oct 11, 2023
7 of 8 checks passed
@fgalan fgalan deleted the task/remove_autocast2 branch October 11, 2023 15:14
@fgalan fgalan mentioned this pull request Oct 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mapedraza mapedraza mapedraza approved these changes

@fgalan fgalan fgalan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AlvaroVega @mapedraza @jason-fox @fgalan