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 for PGXN #168

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mkgrgis
Copy link

@mkgrgis mkgrgis commented Oct 4, 2023

Add separate 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.

Add separate `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 mkgrgis mentioned this pull request Oct 4, 2023
@vaibhavdalvi93
Copy link

Thanks, @mkgrgis for patch. I did more changes like fixing spelling mistakes, deleting duplicate code, indentation, etc.

Find revised patch. Please find difference between patch provided by you and me. If you're agree with the changes then post this patch or point out the changes you are NOT agree. Thanks.
v2-Refactor-README.md-for-PGXN.txt

@mkgrgis
Copy link
Author

mkgrgis commented Oct 5, 2023

Thanks for the patch, @vaibhavdalvi93 , I am studying the changes.

@mkgrgis
Copy link
Author

mkgrgis commented Oct 5, 2023

@vaibhavdalvi93, SQL indent in Examples in case of CREATE SERVER + CREATE USER MAPPING FOR + CREATE FOREIGN TABLE is common tradition for https://github.com/pgspider/influxdb_fdw#examples , https://github.com/pgspider/sqlite_fdw#examples , and https://github.com/pgspider/griddb_fdw#examples etc. There is no such tradition for UPDATE or DELETE commands. What will better for our README.md? Could you please comment directly in https://github.com/EnterpriseDB/mongo_fdw/pull/168/files (select a line of code and write a question or review)?

Note: I have viewed your patch during mkgrgis@92280f5

@vaibhavdalvi93
Copy link

@mkgrgis , Thanks for your comment on v2 patch provided by me.

Could you please comment directly in https://github.com/EnterpriseDB/mongo_fdw/pull/168/files (select a line of code and write a question or review)?

As there are quite more changes I have done in my patch, I am not finding myself comfortable pointing or commenting out each change directly on files.
Do you have any other comment than below one on patch [v2-Refactor-README.md-for-PGXN.txt]? If yes, could you please share here otherwise I will make it ready for committer after doing some more changes.

SQL indent in Examples in case of CREATE SERVER + CREATE USER MAPPING FOR + CREATE FOREIGN TABLE is common tradition for https://github.com/pgspider/influxdb_fdw#examples , https://github.com/pgspider/sqlite_fdw#examples , and https://github.com/pgspider/griddb_fdw#examples etc. There is no such tradition for UPDATE or DELETE commands.

Thanks.

@mkgrgis
Copy link
Author

mkgrgis commented Dec 11, 2023

I am not finding myself comfortable pointing or commenting out each change directly on files.

No problem, @vaibhavdalvi93 ! Unfortunately in your previous message "patch" is not URL for downloading. If you are about this version https://github.com/EnterpriseDB/mongo_fdw/files/12802661/v2-Refactor-README.md-for-PGXN.txt , I have no other comments. You can discuss this patch with commiter. Anyway I can open new PR after this if there will be some new README.md improvements.

@vaibhavdalvi93
Copy link

Okay. Thank you for your response. I will take it further to the commiter. Thanks, again.

jeevanchalke added a commit that referenced this pull request Jan 4, 2024
The main changes include moving the installation steps to the newly
created INSTALL.md file. Other changes in README.md are for unifying
it with FDW documentation template.

Reported on GitHub through issue #168 by mkgrgis (Михаил).

FDW-665, initial patch by mkgrgis, further revised and improved by
Vaibhav Dalvi.
@mkgrgis
Copy link
Author

mkgrgis commented Jan 17, 2024

Thanks, @vaibhavdalvi93! Could you please begin 2nd round of review? I have seen small differences between our branches and propose a changes.

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