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

Include generated ui files in git repo. #12523

Closed
wants to merge 1 commit into from

Conversation

heppu
Copy link
Contributor

@heppu heppu commented Sep 9, 2021

As described in #12507 when //go:embed is used Go expects those files to be available in repository. If the files are not available that becomes a problem when we try to vendor the package as dependency.

This PR should include two things:

  • commit http/web_ui directory in git repository
  • validate in CI that those files have not been edited manually

For the validation part in CI we could do following:

  1. make static-dist
  2. git diff --name-status --exit-code -- ./http/web_ui

I'm not too familiar with Vault CI pipe and not sure what would the correct place for this so any suggestions would be really helpfull. Ideally the UI files would be already generated in CI pipe and we would only need to check with git command that nothing actually changed.

When //go:embed is used Go expects those files to be available in
repository.
@hashicorp-cla
Copy link

hashicorp-cla commented Sep 9, 2021

CLA assistant check
All committers have signed the CLA.

@pmmukh
Copy link
Contributor

pmmukh commented Sep 9, 2021

Hi @heppu ! Thanks for raising this PR! After taking a look at it, we don't feel like having these files added to Vault is a path we want to go down, with 70 something files it causes unnecessary bloat to the repo to support something we don't intend to support ( vendoring vault as a go module ). As such, I'm going to go ahead and close this PR out.

@pmmukh pmmukh closed this Sep 9, 2021
@heppu
Copy link
Contributor Author

heppu commented Sep 9, 2021

Sure I can understand that. @pmmukh How would you feel about having that folder with single place holder file? That way it wouldn't bloat the repo, no need to modify tooling and vendoring would work?

@pmmukh
Copy link
Contributor

pmmukh commented Sep 9, 2021

That honestly seems like a bit of a hacky workaround, and not something we'd like to keep around. I would recommend that for now you depend on an older version of vault that works for vendoring. If you need to both use go 1.17 (as I saw mentioned in your issue) and use vault as a vendored dependency, I'm sorry but I don't think that will be possible.

@heppu heppu deleted the include-webui-files-in-repo branch January 13, 2022 11:16
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.

None yet

3 participants