-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/ZCS 10534 #1
base: zimbra9/onlyoffice
Are you sure you want to change the base?
Feature/ZCS 10534 #1
Conversation
…rvice/spellchecker/coverter; https for docservice; change in connection limit; mysql as db
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.
@sreejan-chowdhury Should you:
- create a working tag base branch in this repo from v6.3.1.10
- set this PR base branch to that working tag branch instead of master
- once pr approved and merged: merge that working tag branch (which now includes these changes) to master
- If the local changes were on top of the git history I would've suggested just rebasing master to latest stuff, then cherry-picking your commits to keep local changes in this repo on top
Less changes for reading.
For reviewers these appear to be local changes:
https://github.com/ONLYOFFICE/server/compare/v6.3.1.10...sreejan-chowdhury:feature/ZCS-10534?expand=1
My comments are not a full review, just casual glance.
Common/sources/commondefines.js
Outdated
@@ -852,7 +908,9 @@ const c_oPublishType = { | |||
meta: 11, | |||
forceSave: 12, | |||
closeConnection: 13, | |||
changesNotify: 14 | |||
changesNotify: 14, | |||
changeConnecitonInfo: 15, |
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.
typo (this appears to be from the tag, and not you)
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.
This is from the tag
Common/sources/license.js
Outdated
@@ -32,28 +32,18 @@ | |||
|
|||
'use strict'; | |||
|
|||
const config = require('config'); | |||
const configL = config.get('license'); |
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.
sketchy file changes 😂 (this appears to be from the tag, and not you)
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.
This is from the tag
SpellChecker/sources/server.js
Outdated
@@ -93,9 +93,9 @@ if (cluster.isMaster) { | |||
if (config.has('ssl')) { | |||
const privateKey = fs.readFileSync(config.get('ssl.key')).toString(); | |||
const certificateKey = fs.readFileSync(config.get('ssl.cert')).toString(); | |||
const trustedCertificate = fs.readFileSync(config.get('ssl.ca')).toString(); | |||
//const trustedCertificate = fs.readFileSync(config.get('ssl.ca')).toString(); |
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.
Did you intend to remove this commented line?
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.
Left it by mistake. Thanks
bin/configure.py
Outdated
data = json.load(f) | ||
|
||
while True: | ||
docservice_port = input("Enter docservice port number (Press enter to keep default): ") |
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.
Would it be more helpful to show the actual default value (e.g. [default here]
) instead of just mentioning there is one? Same for the other prompts in this file.
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.
This file will no longer be needed.
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.
Better way to do it:
Create a branch from ZimbraOS/zm-onlyoffice tag.
Raise PR against it for better visiblity.
A new base branch from tag v6.3.1.10 has been created for the ease of seeing the changes made by me. |
Similar code is done several times at this new zmonlyofficeconfig file. Check: zm-onlyoffice/bin/zmonlyofficeconfig Lines 123 to 133 in 1a223d3
This is overwriting the same file again with the latest value ( > ) instead of appending values ( >> ), isn't it ? You should end up with some configuration file which do not have the needed keys for this to work properly. |
Changes :
Scripts to install rabbitmq, start and stop docservice/spellchecker/coverter;
https for docservice;
change in connection limit;
mysql as db
Updates from https://github.com/ONLYOFFICE/server.git
The branch has been created from tag : v6.3.1.10
The changes can be found in df31f2d
Steps taken :
Created a fork from onlyoffice/server.
Created a branch from v6.3.1.10.
Made changes in the branch.
Created new upstream for ZimbraOS/zm-onlyoffice
pushed new branch to ZimbraOS/zm-onlyoffice
To build on Ubuntu 14:
1.
git clone https://github.com/ONLYOFFICE/build_tools.git
cd build_tools/tools/linux
In: build_tools/scripts/base.py : def git_update(repo, is_no_errors=False, is_current_dir=False):
Replace and Add the following:
../../out/linux_64/onlyoffice/