-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: add N-API version 6 to table #32829
Conversation
We missed adding version 6 to the compatibility table when we defined version 6. Add it along with the versions that we know will include version 6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My one concern is the scalability of this table as new levels are added. Might be worth refactoring to avoid the table growing too wide?
| v11.x | v11.0.0 | v11.0.0 | v11.0.0 | v11.8.0 | | | | ||
| v12.x | v12.0.0 | v12.0.0 | v12.0.0 | v12.0.0 | v12.11.0 | | | ||
| v13.x | v13.0.0 | v13.0.0 | v13.0.0 | v13.0.0 | v13.0.0 | | | ||
| v14.x | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usual process is to add REPLACEME
placeholders that get replaced in the release commit (i.e. 14.0.0
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richardlau will that do the right thing when it gets backported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that depends on whether the commit is picked from current or master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a matrix adding REPLACEME for all versions will not do what we want. Maybe we should add REPLACEME for the version corresponding to the branch against which the commit is being applied, and hardcoded version values for all the other branches.
This means that when we backport this commit we'll necessarily have to edit it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to break from convention/the usual process if it makes sense and does the right thing. I make a point of pointing out when we do diverge from the norm though so that we do so consciously.
@jasnell will think about how to refactor, but I'd like to land as is, as version 6 is already out and would like to avoid confusion in the short term. |
We missed adding version 6 to the compatibility table when we defined version 6. Add it along with the versions that we know will include version 6. PR-URL: #32829 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>
Landed in 2abec12 |
We missed adding version 6 to the compatibility table when we defined version 6. Add it along with the versions that we know will include version 6. PR-URL: #32829 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>
We missed adding version 6 to the compatibility table when we defined version 6. Add it along with the versions that we know will include version 6. PR-URL: #32829 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>
We missed adding version 6 to the compatibility table when we defined version 6. Add it along with the versions that we know will include version 6. PR-URL: #32829 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>
We missed adding version 6 to the compatibility
table when we defined version 6. Add it along with the
versions that we know will include version 6.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes