Skip to content

Conversation

@jimexist
Copy link
Member

@jimexist jimexist commented May 24, 2021

Which issue does this PR close?

Closes #421.

Rationale for this change

we can use prettier to auto-format .md or even .yaml files and set it as a GitHub action to auto-correct.

What changes are included in this PR?

  • added the GitHub action in dev workflow
  • the second commit is done automatically and pushed by the GitHub action

Are there any user-facing changes?

no user facing changes as prettier would not change semantics of docs

@jimexist
Copy link
Member Author

Now it's automatic! Neat!

@codecov-commenter
Copy link

Codecov Report

Merging #415 (694f093) into master (7359e4b) will not change coverage.
The diff coverage is n/a.

❗ Current head 694f093 differs from pull request most recent head 0507ed0. Consider uploading reports for the commit 0507ed0 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #415   +/-   ##
=======================================
  Coverage   74.85%   74.85%           
=======================================
  Files         146      146           
  Lines       24498    24498           
=======================================
  Hits        18339    18339           
  Misses       6159     6159           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7359e4b...0507ed0. Read the comment docs.

Copy link
Contributor

@msathis msathis left a comment

Choose a reason for hiding this comment

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

Great work @jimexist 👍

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you very much @jimexist -- this would be awesome. I don't think we can use the third party action on this repo, but I think we could bring the code in directly if you wanted to try.

👍

ref: ${{ github.head_ref }}

- name: Prettify code
uses: creyD/prettier_action@v3.3
Copy link
Contributor

Choose a reason for hiding this comment

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

I think apache repos are limited to official github actions or ones that are local to this repo (there were some security issues previously):.

Notice: December 27, 2020: We only allow Actions that are official "Made by GitHub" or local to the Apache org on GitHub, to address a potential security vulnerability. This is an incident-related policy change. We are researching the situation, and the policy may evolve based on what we learn.

https://infra.apache.org/github-actions-secrets.html

Given https://github.com/creyD/prettier_action appears to be MIT licensed, however, what about copying the code for that action into this repo?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point. do you think copying it to be something under apache org repo is desired?

Copy link
Contributor

Choose a reason for hiding this comment

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

good point. do you think copying it to be something under apache org repo is desired?

Yes, I think that would be preferable (the rationale being that then that someone can't change the contents of the action in the third party repository and affect the apache org via a github action)

@alamb
Copy link
Contributor

alamb commented May 25, 2021

FYI @Dandandan

@alamb
Copy link
Contributor

alamb commented Jun 2, 2021

Seems like this is supserceded by #453 so closing this to keep the PR queue down. Please reopen it @jimexist if that is not correct

@alamb alamb closed this Jun 2, 2021
@jimexist jimexist deleted the use-prettier-to-auto-format branch June 10, 2021 01:49
unkloud pushed a commit to unkloud/datafusion that referenced this pull request Mar 23, 2025
* Fixes Issue apache#361: Use enum to represent CAST eval_mode in expr.proto

* Update expr.proto and QueryPlanSerde.scala for handling enum EvalMode for cast message

* issue 361 fixed type issue for eval_mode in planner.rs

* issue 361 refactored QueryPlanSerde.scala for enhanced type safety and localization compliance, including a new string-to-enum conversion function and improved import organization.

* Updated planner.rs, expr.proto, QueryPlanSerde.scala for enum support

* Update spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala

---------

Co-authored-by: Prashant K. Sharma <prakush@foundation.local>
Co-authored-by: Andy Grove <andygrove73@gmail.com>
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.

use a consistent syntax format for md files

4 participants