Skip to content

Module Sharing#352

Merged
Pita merged 9 commits intoether:masterfrom
cweider:modulize-share
Feb 14, 2012
Merged

Module Sharing#352
Pita merged 9 commits intoether:masterfrom
cweider:modulize-share

Conversation

@cweider
Copy link
Copy Markdown
Contributor

@cweider cweider commented Jan 22, 2012

With #343 pulled, a lot of code removal can be done. This branch has the client and server use the same AttributePool, Changeset, and contentcollector modules. For now the common code exists in /static/js/, which is not great, but good enough for now (a later reorganization to move code in to common, pad, ace_inner, and timeline might not be a bad idea).

This change also removes the *_client modules that had duplicates (domline, linestylefilter, and cssmanager). A review of the diffs showed only 2 significant additions existed in the retained modules (followAttributes in easysync2 and the top parameter to getSheetByTitle in cssmanager). All the other changes were whitespace and formatting.

@cweider
Copy link
Copy Markdown
Contributor Author

cweider commented Jan 30, 2012

Bump

@Pita
Copy link
Copy Markdown
Contributor

Pita commented Jan 30, 2012

Can you please fix the merge conflict?

@cweider
Copy link
Copy Markdown
Contributor Author

cweider commented Jan 30, 2012

Rebased

@cweider
Copy link
Copy Markdown
Contributor Author

cweider commented Feb 1, 2012

Just add a change to move randomString into common code.

@Pita
Copy link
Copy Markdown
Contributor

Pita commented Feb 6, 2012

I can has rebase? Sorry that you have to wait so long to get this merged

@cweider
Copy link
Copy Markdown
Contributor Author

cweider commented Feb 6, 2012

Ok, it should be good now.

@cweider
Copy link
Copy Markdown
Contributor Author

cweider commented Feb 10, 2012

Rebased :)

Pita pushed a commit that referenced this pull request Feb 14, 2012
@Pita Pita merged commit 5751a8e into ether:master Feb 14, 2012
@jhollinger
Copy link
Copy Markdown
Contributor

I'm getting ReferenceError: randomstring is not defined from db/Pad.js:482, which I think is related to these changes. I think the API setPassword set it off. Can anyone confirm?

@cweider
Copy link
Copy Markdown
Contributor Author

cweider commented Feb 14, 2012

Ah, yes, that’s a typo. I’ll push a fix in a few.

@cweider
Copy link
Copy Markdown
Contributor Author

cweider commented Feb 14, 2012

@jhollinger fixed with 9837cda.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants