Update the list of modules currently supported by Fennec

RESOLVED FIXED

Status

Add-on SDK
Documentation
P1
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: zer0, Assigned: zer0)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
We're working to supports Fennec, as results of this effort the list of modules currently supported by Fennec is increased. The documentation should be updated.
(Assignee)

Updated

6 years ago
Assignee: nobody → zer0
(Assignee)

Updated

6 years ago
Blocks: 789750
(Assignee)

Updated

6 years ago
No longer blocks: 789750
(Assignee)

Updated

6 years ago
Duplicate of this bug: 789750

Updated

6 years ago
Priority: -- → P1
Is the pull request ready for review?
Flags: needinfo?(zer0)
(Assignee)

Comment 3

6 years ago
Created attachment 696313 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/704

Pointer to Github pull-request
Flags: needinfo?(zer0)
(Assignee)

Updated

6 years ago
Attachment #696313 - Flags: review?(wbamberg)
(Assignee)

Comment 4

6 years ago
Now yes: I close the old pull request in favor of a better one, with a comprehensive list of all modules: I assigned the review to Will.
Comment on attachment 696313 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/704

I'm a bit uneasy about the extra machinery needed here, especially all the handcoded HTML, but it does look prettier.

I'm also worried it won't get kept up to date, but let's look at automating it when we have metadata for application compatibility.

There are also some language changes, and some HTML tags not closed, for which I've made specific comments in the PR.
Attachment #696313 - Flags: review?(wbamberg) → review-
(Assignee)

Comment 6

5 years ago
I don't like the all HTML code use here as well, but I didn't find a better way to list all the modules without makes the page unreadable and too long. As you said (and I thought I wrote that as well somewhere), I hope that have module metadata with application's compatibility will give to us the capability to generate this list automatically. There is already a `<ul id="modules-index">` or similar, that is doing something like that.

This is to me something temporary, that is much better for the developers than the poor list we have at the moment.
(Assignee)

Comment 7

5 years ago
Will, I updated the pull with the comment you suggest. Shall I create a new attachment, even if the link is the same, or you will use the same to approve / keep rejected?
Flags: needinfo?(wbamberg)
Thanks for making this changes Matteo.

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #7)
> Will, I updated the pull with the comment you suggest. Shall I create a new
> attachment, even if the link is the same, or you will use the same to
> approve / keep rejected?

I don't think it matters, whichever you prefer.
Flags: needinfo?(wbamberg)
(Assignee)

Comment 9

5 years ago
In that case, I'd like to reuse the same attachment, you can change the status to approve it – or add a comment with the current reject status explained what is still missed.

Thanks, Will!

Comment 10

5 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/b6ba45304bd9d9fd5b750273b93b30fe15e25a26
Bug 807277 - Update the list of modules currently supported by Fennec

- Added modules' compatibility list on documentation
- Updated css

https://github.com/mozilla/addon-sdk/commit/f8381fd23b05868deef7b90de8a607a1a835038e
Merge pull request #704 from ZER0/mobile-list/807277

Bug 807277 - Update the list of modules currently supported by Fennec
Comment on attachment 696313 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/704

Thanks Matteo!
Attachment #696313 - Flags: review- → review+

Updated

5 years ago
Depends on: 827614
Looks like some link isn't working right.
(Assignee)

Comment 13

5 years ago
Yeah, I saw this morning: in the branch the content-proxy.md was still there, so all the test was passing, and because it was just deleted in master github didn't said anything about conflict – of course, it's a "runtime conflict".
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Exactly. Is there something different we should do to avoid problems like this? When I landed code by applying patches, I'd always test after applying the patch to master but before pushing the updated master, so would have caught something like this. But with a pull request, I generally don't. Should I?
(In reply to Will Bamberg [:wbamberg] from comment #14)
> Exactly. Is there something different we should do to avoid problems like
> this? When I landed code by applying patches, I'd always test after applying
> the patch to master but before pushing the updated master, so would have
> caught something like this. But with a pull request, I generally don't.
> Should I?

For something that is large and touches a lot of the code it is probably worth it. For other code (like this change in fact) I probably wouldn't bother. The automated tests tell us when something has gone wrong and they caught this error.
You need to log in before you can comment on or make changes to this bug.