Skip to content
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

Refactor README.md #165

Closed
wants to merge 12 commits into from
Closed

Conversation

mkgrgis
Copy link

@mkgrgis mkgrgis commented Jan 21, 2023

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.

Remove long instructions from README.md
Add INSTALL.md.
Other changes 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
Copy link
Author

mkgrgis commented Sep 28, 2023

@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
Copy link

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
Copy link
Author

mkgrgis commented Oct 4, 2023

Could you please re-base the patches.

No problems, @vaibhavdalvi93, but I am still not familiar with git. Look like this mkgrgis@6911429 is enough or not?

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

Thanks, fixed in b77aa40 . This is for INSTALL.md.

@mkgrgis
Copy link
Author

mkgrgis commented Oct 4, 2023

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

@vaibhavdalvi93
Copy link

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 9bfc78d
Author: Jeevan Chalke [email protected]
Date: Fri Jul 14 15:30:25 2023 +0530

Stamp 5.5.1. 

Hope, it's clear now.

@mkgrgis
Copy link
Author

mkgrgis commented Oct 4, 2023

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

@vaibhavdalvi93
Copy link

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
Copy link
Author

mkgrgis commented Oct 4, 2023

@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
Copy link
Author

mkgrgis commented Oct 4, 2023

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

@vaibhavdalvi93
Copy link

@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
Copy link
Author

mkgrgis commented Oct 4, 2023

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

@mkgrgis
Copy link
Author

mkgrgis commented Oct 4, 2023

@vaibhavdalvi93, please see accumulated and squashed #168. If all is ok, I'll close this PR.

@vaibhavdalvi93
Copy link

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

@mkgrgis
Copy link
Author

mkgrgis commented Oct 5, 2023

Thanks for review, @vaibhavdalvi93

@mkgrgis mkgrgis closed this Oct 5, 2023
@mkgrgis mkgrgis deleted the readme_refactor branch October 5, 2023 03:49
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.

2 participants