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

Git-ignore files from mix assets.deploy #2789

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jyeshe
Copy link
Member

@jyeshe jyeshe commented Dec 17, 2024

Description

This PR fixes the git ignore on Lightning for files generated by mix assets.deploy

Closes #2788

Validation steps

  1. Run mix assets.deploy (before and after)

Additional notes

  • This needs to be fixed also in the billing app but this one is for Lightning standalone (please see the state on the issue).
  • .gitignore does not support {.ext1, ext2} notation

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed a self-review of my code.
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

Copy link

codecov bot commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.27%. Comparing base (ad06c03) to head (157a0ab).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2789   +/-   ##
=======================================
  Coverage   91.27%   91.27%           
=======================================
  Files         336      336           
  Lines       11946    11946           
=======================================
  Hits        10904    10904           
  Misses       1042     1042           

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

@jyeshe jyeshe force-pushed the gitignore-assets-deploy branch from ca12d44 to 157a0ab Compare December 17, 2024 10:23
@jyeshe jyeshe self-assigned this Dec 17, 2024
Copy link
Member

@taylordowns2000 taylordowns2000 left a comment

Choose a reason for hiding this comment

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

I suppose this is fine, but why have I never had any issues with these files before? Is mix assets.deploy actually used in any part of developing Lightning?

@jyeshe
Copy link
Member Author

jyeshe commented Dec 17, 2024

I suppose this is fine, but why have I never had any issues with these files before? Is mix assets.deploy actually used in any part of developing Lightning?

This is not an issue per se but there were recurring requests that these files should be gitignored on the billing app. When the assets are deployed on the umbrella app, it invokes all apps with the same command (except for mix test which was customised)

The mix assets.deploy is needed on releases and when it's done via Dockerfile we don't see them locally.

At last, the billing app can probably work now without digested deployment but this PR helps to avoid showing those files again (a following will be provided for the billing app).

Copy link
Collaborator

@midigofrank midigofrank left a comment

Choose a reason for hiding this comment

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

Hey @jyeshe, great job!.

I'm worried about these two though:

/priv/static/images/*-*.png
/priv/static/images/*-*.svg 

We already have some images in there that were manually placed in there. Also, if we ever want to add a new image, where would we put it?

@jyeshe
Copy link
Member Author

jyeshe commented Dec 18, 2024

Hey @jyeshe, great job!.

I'm worried about these two though:

/priv/static/images/*-*.png
/priv/static/images/*-*.svg 

We already have some images in there that were manually placed in there. Also, if we ever want to add a new image, where would we put it?

Thanks Frank, that's correct. Repo-wise, the new ones are always ignored in favour of saved ones (the release goes along with the digest) and devs need to intentionally replace them with git add -f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

Git ignore the assets
3 participants