Closed Bug 505316 Opened 15 years ago Closed 15 years ago

Embedders and users should not have to know about API versioning

Categories

(Tamarin Graveyard :: Virtual Machine, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
flash10.1

People

(Reporter: jodyer, Assigned: jodyer)

References

Details

Attachments

(1 file)

API versioning is an optional (and complex) feature that currently requires an updated asc.jar and modifications to host embedding code (ShellCore). Let's not force such a change on those who don't want it. Requirments: - allow TR to work with any asc.jar that FP10 works with - allow TR to work without any (versioning related) modifications to embedding code
Assignee: nobody → jodyer
Blocks: 472920
Attached patch first candidate (deleted) — Splinter Review
this patch set api versioning defaults in AvmCore so that ShellCore doesn't have to. The defaults are that there is just one version (0) and no versioned namespaces. The result is that no names (or namespaces) are marked with a version number. (This patch requires that https://bugzilla.mozilla.org/attachment.cgi?id=389500&action=edit has already been applied.)
Attachment #389571 - Flags: superreview?(edwsmith)
Attachment #389571 - Flags: review?(lhansen)
Comment on attachment 389571 [details] [diff] [review] first candidate Two style nits: Both apis and apis_sizes should be defined as const arrays, since I believe you don't want to update them. For the call to setAPIInfo, instead of passing '1' as the third parameter it is customary to pass sizeof(avmplus::apis)/sizeof(avmplus::apis[0]). That way a change to the array does not need to be accomodated by changes to the code. Ditto for any other parameters in this list to which that might apply.
Attachment #389571 - Flags: review?(lhansen) → review+
+1 to Lars minor nits, otherwise, looks good to me too.
Fixes of nits have been integrated into the latest patch of bug 472920. In Ed's absence, will take his super review there and steven's review here as SR+ for this patch (unless I hear that I should do otherwise.)
Status: NEW → ASSIGNED
(In reply to comment #4) > Fixes of nits have been integrated into the latest patch of bug 472920. In Ed's > absence, will take his super review there and steven's review here as SR+ for > this patch (unless I hear that I should do otherwise.) Make it so.
pushed: changeset - 2212:5d74ffcdc709
QRB: target for current milestone QE: in-testsuite:? since we should do a one off test using an pre-"API versioning" asc.jar
Flags: in-testsuite?
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
Flags: flashplayer-qrb? → flashplayer-qrb+
Target Milestone: --- → flash10.1
Attachment #389571 - Flags: superreview?(edwsmith)
Jeff: if it's been pushed, can we close?
Priority: -- → P2
Pushed (see Comment #6)
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Testing with pre-api versioning done.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: