Vauxoo / pre-commit-vauxoo

pre-commit-vauxoo python library to add a command to use all the configuration files and environment variables of Vauxoo
Other
2 stars 2 forks source link

Do not remove commas in scss files #18

Closed rolandojduartem closed 2 years ago

rolandojduartem commented 2 years ago

@moylop260

The following issue happend in the recent MR where some commas like this and this were removed using pre-commit-vauxoo. Removing those commas caused this traceback which affect tour tests like this. The problem was solved putting back those commas, but if you try to execute pre-commit-vauxoo again, it will remove those commas again. This is important for the new lint to avoid reformatted issues.

Issue caused by removing commas:

Screenshot from 2022-08-24 15-34-32

Screenshot from 2022-08-24 15-48-03

Resolved putting back commas (manually):

Screenshot from 2022-08-24 15-44-34

cc @luisg123v

moylop260 commented 2 years ago

@rolandojduartem

I can not reproduce it

steps to reproduce

  1. git clone --depth=1 -b 14.0-instance-black_isort_prettier-t54296-dev-rolando git@git.vauxoo.com:vauxoo-dev/instance.git
  2. cd instance
  3. PRECOMMIT_AUTOFIX=1 pre-commit-vauxoo && PRECOMMIT_AUTOFIX=1 pre-commit-vauxoo
  4. git status

Expected

See the scss changes

Saw

    modified:   CHANGELOG
    modified:   CONTRIBUTING.md
    modified:   check_doc.sh
    modified:   post-migration-script.sql

Could you describe better the steps to reproduce it, please?

moylop260 commented 2 years ago

FYI I have already reproduced using the branch 14.0 sha b14bac0757807b24336930f886ba3693d3447587

After the configuration files were copied the issue is reproduced faster running the following command: pre-commit run prettier --color=always -c .pre-commit-config-autofix.yaml --file theme_vauxoo/static/src/scss/primary_variables.scss

moylop260 commented 2 years ago

@rolandojduartem

Could you check if the output from https://github.com/Vauxoo/pre-commit-vauxoo/pull/19 is raising error too, please?

rolandojduartem commented 2 years ago

@moylop260 I followed this conversation in the PR FYI

luisg123v commented 2 years ago

I think this is because of https://github.com/prettier/prettier/issues/8855