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.

Sign up for GitHub

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

Jump to bottom

Update readme #39

Merged
merged 16 commits into from
May 17, 2021
Merged

Update readme #39

merged 16 commits into from
May 17, 2021

Conversation

Andre601
Copy link
Member

Thank you for contributing! Please check the following things before submiting your PR:

Required:

If necessary:

  • I have updated the README and documentation.
  • I have updated the ChangeLog with the changes I have made.

Description

Adds a section about the different Comment options you can use in markdown files.

Closes #

Note: we will close your PR without comment if you do not check the required boxes above and provide ALL requested information.

@Andre601 Andre601 marked this pull request as draft May 16, 2021 15:57
@PuneetGopinath PuneetGopinath added Status: In progress This pr or issue is in progress minor Will be added to next minor release labels May 17, 2021
@PuneetGopinath PuneetGopinath mentioned this pull request May 17, 2021
Merged
3 tasks
@PuneetGopinath
Copy link
Member

@Andre601 Is this pr ready for review?

@Andre601 Andre601 added the Status: Ready The Pull request is ready for reviews and getting merged label May 17, 2021
@Andre601 Andre601 marked this pull request as ready for review May 17, 2021 09:06
@github-actions
Copy link
Contributor

The Pull request of @Andre601 has been marked as Ready!

It can now be reviewed and/or merged by Maintainers.

This is an automated response created by a GitHub Action
Mentioning the bot won't have any effect!

@PuneetGopinath
Copy link
Member

@abhijoshi2k Please review

@Andre601 Andre601 removed the Status: In progress This pr or issue is in progress label May 17, 2021
@PuneetGopinath PuneetGopinath changed the title Update readme Update readme and changelog May 17, 2021
@Andre601 Andre601 changed the title Update readme and changelog Update readme May 17, 2021
@Andre601
Copy link
Member Author

The changelog is handled in #41 :)

@abhijoshi2k abhijoshi2k added the Updates: documentation Improvements or additions to documentation label May 17, 2021
@abhijoshi2k
Copy link
Member

@Andre601 @PuneetGopinath
Can we remove RECENT_ACTIVITY:end and RECENT_ACTIVITY:last_update_end from the Options table?
It might confuse the users. Also, users don't have to add it themselves. Workflow will take care of it.
I can see that you clearly mentioned this fact. But I still feel we should remove those rows.

@PuneetGopinath
Copy link
Member

The changelog is handled in #41 :)

Ok

@PuneetGopinath
Copy link
Member

@Andre601 @PuneetGopinath
Can we remove RECENT_ACTIVITY:end and RECENT_ACTIVITY:last_update_end from the Options table?
It might confuse the users. Also, users don't have to add it themselves. Workflow will take care of it.
I can see that you clearly mentioned this fact. But I still feel we should remove those rows.

Oh yes, you are right
@Andre601 Can you remove I am little busy.

@abhijoshi2k
Copy link
Member

abhijoshi2k commented May 17, 2021 •
edited
Loading

@Andre601 Can we add a Wiki to show supported placeholders in date format?
f.e. dd/mm/yyyy or dddd mmmm ds etc.

Supported formats: https://www.npmjs.com/package/dateformat

We support the masked options as well as named formats

@abhijoshi2k
Copy link
Member

@Andre601 Can we add a Wiki to show supported placeholders in date format?

Supported formats: https://www.npmjs.com/package/dateformat

We support the masked options as well as named formats

You can get raw README of that package on this repo: https://github.com/felixge/node-dateformat

@Andre601
Copy link
Member Author

@Andre601 Can we add a Wiki to show supported placeholders in date format?

Supported formats: https://www.npmjs.com/package/dateformat

We support the masked options as well as named formats

Added an example

@abhijoshi2k
Copy link
Member

I was thinking we can add something like this:
image

@abhijoshi2k
Copy link
Member

I was thinking we can add something like this:

It's available here : https://github.com/felixge/node-dateformat

@Andre601
Copy link
Member Author

Not worth to add this info imo if it already exists. I already link to the npm package and that should be enough.

You should better try to fix the conflicts in this PR because I won't touch JS stuff here.

@abhijoshi2k
Copy link
Member

Not worth to add this info imo if it already exists. I already link to the npm package and that should be enough.

You should better try to fix the conflicts in this PR because I won't touch JS stuff here.

Okay. I'll resolve the conflicts.
Should I merge after that?

@PuneetGopinath
Copy link
Member

PuneetGopinath commented May 17, 2021 •
edited
Loading

Not worth to add this info imo if it already exists. I already link to the npm package and that should be enough.
You should better try to fix the conflicts in this PR because I won't touch JS stuff here.

Okay. I'll resolve the conflicts.
Should I merge after that?

No, after resolving conflicts you should pull branch updation-time into update-readme.

@PuneetGopinath
Copy link
Member

@abhijoshi2k Better delete node_modules folder.
I messaged you in discord that "Yes, I afterwards saw in GitHub docs that if you use ncc then you should remove node_modules folder.
We are using @vercel/ncc npm package"

@Readme-Workflows Readme-Workflows deleted a comment from Andre601 May 17, 2021
@PuneetGopinath
Copy link
Member

Better I itself will delete, because you started working and you are not going to see my comment

@abhijoshi2k
Copy link
Member

Better I itself will delete, because you started working and you are not going to see my comment

The bot added it back

@PuneetGopinath
Copy link
Member

Better I itself will delete, because you started working and you are not going to see my comment

The bot added it back

Yeah, I see I will force push

@abhijoshi2k
Copy link
Member

Better I itself will delete, because you started working and you are not going to see my comment

The bot added it back

Yeah, I see I will force push

I think it's fine if we have node_modules. Only the cache files cause conflicts which can be ignored

@PuneetGopinath
Copy link
Member

Why? GitHub docs itself says to remove it and and node_modules to .gitignore

@abhijoshi2k
Copy link
Member

Idk why, but I don't see any conflicts on local. Should I merge this into updation-time then?

@PuneetGopinath PuneetGopinath merged commit 1c6e8d8 into updation-time May 17, 2021
@PuneetGopinath PuneetGopinath deleted the update-readme branch May 17, 2021 10:48
@Andre601
Copy link
Member Author

Conflicts seem resolved. Just the action was failing for some reason, but I leave this up to you to figure out

@PuneetGopinath
Copy link
Member

They fail because there is nothing to commit, or no changes happen to the files when you run npm run build and npm run format.

PuneetGopinath added a commit that referenced this pull request May 17, 2021
@PuneetGopinath @github-actions @Andre601
Merge pull request #40 from Readme-Workflows/updation-time
8ab0583 
* add date option

* run build and format

* add dateformat package

* add dateformat package

* run build and format

* removed no changes detected

* run build and format

* removed timezone bug

* run build and format

* removed timezone bug

* run build and format

* Update index.js

* run build and format

* added logging statement

* run build and format

* Update index.js

* run build and format

* added multiline DATE_STRING support

* run build and format

* Update readme (#39)

* Update README.md
* Improve table
* Add TIMEZONE_OFFSET to readme
* Add DATE_STRING and {DATE} placeholder
* Add DATE_FORMAT to readme
* Improve descriptions
* Update README.md

Co-authored-by: Puneet Gopinath <[email protected]>
Co-authored-by: PuneetGopinath <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* update package-lock.json

* run prettier and build

* run build and format

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: PuneetGopinath <[email protected]>
Co-authored-by: Andre_601 <[email protected]>
Co-authored-by: Puneet Gopinath <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PuneetGopinath PuneetGopinath PuneetGopinath approved these changes

@abhijoshi2k abhijoshi2k abhijoshi2k approved these changes

Assignees
No one assigned
Labels
minor Will be added to next minor release Status: Ready The Pull request is ready for reviews and getting merged Updates: documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Andre601 @PuneetGopinath @abhijoshi2k