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

add an empty index.html under core/web/assets to avoid running make #11114

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

shileiwill
Copy link
Contributor

@shileiwill shileiwill commented Oct 27, 2023

AUTO-6903

Context:

I am working on this debugging script for our customers which is triggered by cli, e.g. cd to core/script/chaincli and then go run main.go keeper debug 96601733428484125246719907842312606900925034674711094396763910645276293996221 0x5a51a2cf2283b1d4a075011c5e51308d4a34638e3ca00accc5f5ed2a71633ffe 5

The problem is: When running the above command, it complains ../../web/middleware.go:24:12: pattern assets: no matching files found . Digging in, I figured it is from this //go:embed "assets" directive in middleware.go. (I got your name from the history of this file 🙂 )

Solutions are:

  1. we can run make test_need_operator_assets and generate the index.html under core/web/assets, but this is not preferred as customers need to run this manually.
  2. how about we create an empty index.html under asserts/, so whenever customers get the chainlink repo, it has index.html?
  3. is it possible to skip the go:embed at all for our usecase?

This PR:
Per talking to Makram, there is no option 3, they've been using this make command make operator-ui. So, to avoid letting users run a unrelated make command, we go with option 2, which is this PR to add an empty index.html file to bypass the check.

@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@RyanRHall
Copy link
Contributor

I absolutely love option 2. Do we need to change any .gitignore files or anything? Will this change commit future changes to index.html or will it remain as an empty file? Can we remove that make command now?

@shileiwill
Copy link
Contributor Author

Thanks @RyanRHall !

Do we need to change any .gitignore files or anything?
I dont think so, this core/web/asset directory is ignored. I force added this index.html in this PR, but the gitignore will still in effect.

Will this change commit future changes to index.html or will it remain as an empty file?
It shouldn't commit future changes, it will remain an empty file unless there are other commands override it, as far as i understand.

Can we remove that make command now?
I think this [test_need_operator_assets](https://github.com/smartcontractkit/chainlink/blame/5e1c3a3f2ddf35ac6e86f9640ba33f71173efadc/GNUmakefile#L130) make command can be remove since we always have a default index.html now. cc @AnieeG who added this command 1 year ago :) Appreciate Ani if you can help take a look :)

@shileiwill
Copy link
Contributor Author

shileiwill commented Nov 1, 2023

Confirmed with Ani that we can remove the test_need_operator_assets make command and its refererences. cc @RyanRHall @AnieeG

@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@shileiwill shileiwill added this pull request to the merge queue Nov 1, 2023
Merged via the queue into develop with commit fda4393 Nov 1, 2023
84 checks passed
@shileiwill shileiwill deleted the index_html branch November 1, 2023 19:55
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.

3 participants