EnterpriseDB / mongo_fdw

PostgreSQL foreign data wrapper for MongoDB
GNU Lesser General Public License v3.0
326 stars 70 forks source link

Refactor README.md #165

Closed mkgrgis closed 12 months ago

mkgrgis commented 1 year ago

Add INSTALL.md. Other changes in README.md for unifying with https://github.com/pgspider FDW documentation template and by https://github.com/ibarwick/firebird_fdw/blob/master/README.md example of advanced FDW documentation.

mkgrgis commented 1 year ago

@jeevanchalke, ping. Have you got any time to review this PR with refactoring README.md to most widespread text order and typography ? It will be very good if release for PG 16 will have unified README.md for PGXN.

vaibhavdalvi93 commented 1 year ago

Thanks, @mkgrgis for the patches. I am getting below conflicts on latest master.

localhost:mongo_fdw$ git apply ~/Downloads/165.patch
/home/vaibhav/Downloads/165.patch:613: trailing whitespace.

/home/vaibhav/Downloads/165.patch:717: trailing whitespace.

/home/vaibhav/Downloads/165.patch:745: trailing whitespace.

/home/vaibhav/Downloads/165.patch:850: trailing whitespace.

/home/vaibhav/Downloads/165.patch:855: trailing whitespace.

error: patch failed: README.md:8
error: README.md: patch does not apply
error: patch failed: README.md:1
error: README.md: patch does not apply
error: patch failed: README.md:466
error: README.md: patch does not apply
error: patch failed: README.md:429
error: README.md: patch does not apply
error: patch failed: README.md:5
error: README.md: patch does not apply

Could you please re-base the patches.

Also, please check on reference number 5 and 6 given in INSTALL.md.

mkgrgis commented 1 year ago

Could you please re-base the patches.

No problems, @vaibhavdalvi93, but I am still not familiar with git. Look like this https://github.com/mkgrgis/mongo_fdw/commit/6911429b5c1bb2e19daf4b12cc60742111396ecc is enough or not?

Also, please check on reference number 5 and 6 given in INSTALL.md.

Thanks, fixed in https://github.com/EnterpriseDB/mongo_fdw/pull/165/commits/b77aa4087a6270ccf55fe9d162c5b224384012c3 . This is for INSTALL.md.

mkgrgis commented 1 year ago

Also, @vaibhavdalvi93, You can squash my commits before merging. It will look more better and there will no unnecessary git information.

vaibhavdalvi93 commented 1 year ago

Regarding re-basing the changes, the changes you have done in README.md are not on latest master. E.g. In the current README file, we have mentioned about supporting version 16 as well as shown below:

Please note that this version of mongo_fdw works with PostgreSQL and EDB Postgres Advanced Server 11, 12, 13, 14, 15, and 16.

Also, new GUC's (i.e. mongo_fdw.enable_join_pushdown and mongo_fdw.enable_aggregate_pushdown) and table, server level options (i.e. enable_order_by_pushdown).

As I said earlier, please check the latest master. The latest commit is

commit 9bfc78d52bd6dfe6b51ff3eee6c7be4d93d513f4 Author: Jeevan Chalke jeevan.chalke@enterprisedb.com Date: Fri Jul 14 15:30:25 2023 +0530

Stamp 5.5.1. 

Hope, it's clear now.

mkgrgis commented 1 year ago

@vaibhavdalvi93 , please ensure I have forget noting from https://github.com/EnterpriseDB/mongo_fdw/commits/master/README.md Thanks for notes! I think in https://github.com/EnterpriseDB/mongo_fdw/pull/165/commits/44090b30bf30f31511be77608df8dc5f00d762da there is fixing to documentation additions before https://github.com/EnterpriseDB/mongo_fdw/commit/9bfc78d52bd6dfe6b51ff3eee6c7be4d93d513f4

vaibhavdalvi93 commented 1 year ago

Thanks, @mkgrgis. These changes are fine but why can't you take latest master and prepare PR on top of it? If I apply this patch on latest master then it created too many conflicts. The committer is not going to checkout on old commit and apply your patches. Thoughts? If you are agree, please prepare the patches on top of current HEAD. Deleting existing changes and then add refactoring then re-apply deleted changes doesn't make sense to me. Correct me, if I'm wrong here.

mkgrgis commented 1 year ago

@vaibhavdalvi93 , I am studying how to rebase to current HEAD. I am not familiar with git. I have got actual https://github.com/mkgrgis/mongo_fdw/tree/master branch but I don't know how to rebase without PR recreating. I know this is possible, maybe you can advice me some command after git rebase?

mkgrgis commented 1 year ago

I am doing git rebase 9bfc78d52bd6dfe6b51ff3eee6c7be4d93d513f4. Look like this is what I need.

vaibhavdalvi93 commented 1 year ago

@mkgrgis , I don't think there is direct command which is going to help here to re-base. You can use below command and the hunks which will fail, you need to add those manually.

patch -p1 < 165.patch

mkgrgis commented 1 year ago

@vaibhavdalvi93 , I'll recreate PR during few minutes. You can ensure the same README.md and INSTALL.md with diff.

mkgrgis commented 1 year ago

@vaibhavdalvi93, please see accumulated and squashed https://github.com/EnterpriseDB/mongo_fdw/pull/168. If all is ok, I'll close this PR.

vaibhavdalvi93 commented 1 year ago

Please close this pull request as #168 is opened for the same.

mkgrgis commented 12 months ago

Thanks for review, @vaibhavdalvi93