Skip to content

Conversation

LamJiuFong
Copy link
Contributor

@LamJiuFong LamJiuFong commented Mar 17, 2024

What is the purpose of this pull request?
Fixes #2416

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
Rename .gitignore to gitignore in template/default and rename gitignore to .gitignore when user run markbind-cli init

Anything you'd like to highlight/discuss:
I have tested it locally, but I think we can only know if this method actually works until we publish the package on npm, not sure if we are allowed to publish a draft package?

Testing instructions:
Make the changes in this PR and npm link an empty folder to the local markbind-cli
Ensure that local markbind-cli is linked to local @markbind-core
eg,
In the packages/cli directory,
Run npm ls --link , should see @markbind/[email protected] -> ./packages/core

In the empty folder, run npx markbind-cli init, a .gitignore file will be created

Proposed commit message: (wrap lines at 72 characters)
Change .gitignore of the default template to gitignore

The default template has a .gitignore file

.gitignore will be ignored by npm and is not able to be published.
The file will not be created in the users' project.

Let's rename it to gitignore so that it can be published and rename
it back to .gitignore in the users' project.


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

Copy link

codecov bot commented Mar 17, 2024

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 51.20%. Comparing base (dd4bfd3) to head (2a01bb0).

Files Patch % Lines
packages/core/src/utils/fsUtil.ts 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2469      +/-   ##
==========================================
- Coverage   51.22%   51.20%   -0.02%     
==========================================
  Files         124      124              
  Lines        5281     5283       +2     
  Branches     1121     1122       +1     
==========================================
  Hits         2705     2705              
- Misses       2290     2291       +1     
- Partials      286      287       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@yucheng11122017 yucheng11122017 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work @LamJiuFong and for the investigation as well!

Generally looks good except for some minor nits.

*.iml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how to fix this problem because the changes were made by WebStorm (due to missing EoF line in the original code), I tried using GitHub to remove the last line but nothing happened d62c7ac

@kaixin-hc
Copy link
Contributor

formatting still got issue ah!

that aside, great investigation and thanks for finding a fix so quickly : )

@LamJiuFong
Copy link
Contributor Author

Thanks @kaixin-hc @yucheng11122017 for reviewing!

Copy link
Contributor

@yucheng11122017 yucheng11122017 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thanks!

Copy link
Contributor

@kaixin-hc kaixin-hc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm ! thank u

@kaixin-hc
Copy link
Contributor

@LamJiuFong Can you write a draft PR message that includes the explanation for why this fixes a bug? Thanks!

@LamJiuFong
Copy link
Contributor Author

@kaixin-hc I have updated the commit message, is that okay?

@kaixin-hc kaixin-hc merged commit 2658da9 into MarkBind:master Mar 20, 2024
@kaixin-hc kaixin-hc added this to the v5.4.1 milestone Apr 6, 2024
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.

.gitignore not generated when init
3 participants