Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add option to add files in container #11

Merged
merged 3 commits into from
Jun 20, 2024
Merged

add option to add files in container #11

merged 3 commits into from
Jun 20, 2024

Conversation

mberndt123
Copy link
Contributor

@mberndt123 mberndt123 commented Jun 5, 2024

Hi,

I've added the possibility to add files that will be mounted into the scala-steward container. I needed this for two things:

  • for use with the --repo-config option which requires a file. I could of course put this into each individual repo, but I want to be able to configure it the same way across repos
  • for the /root/.sbt/1.0/credentials.sbt file where I read the Nexus credentials from the env variables. again, I don't want to modify lots of different repos to do this.

Let me know what you think.

Copy link
Collaborator

@jsoref jsoref left a comment

Choose a reason for hiding this comment

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

This looks interesting. As it happens, we planned to look into upgrading scala steward this month...

charts/scala-steward/values.yaml Outdated Show resolved Hide resolved
charts/scala-steward/templates/configmap.yaml Outdated Show resolved Hide resolved
charts/scala-steward/templates/cron-job.yaml Show resolved Hide resolved
charts/scala-steward/templates/cron-job.yaml Show resolved Hide resolved
charts/scala-steward/values.yaml Outdated Show resolved Hide resolved
charts/scala-steward/values.yaml Outdated Show resolved Hide resolved
@mberndt123
Copy link
Contributor Author

mberndt123 commented Jun 5, 2024

Thanks for the review, I've rebased the changes and also adjusted the version from latest-release to 0.30.2.

@mberndt123
Copy link
Contributor Author

I've also adjusted version and appVersion in Chart.yaml

@mberndt123
Copy link
Contributor Author

@jsoref can you take another look?

@jsoref
Copy link
Collaborator

jsoref commented Jun 5, 2024

Thanks! The changes look right. I want to test this a bit before I merge. I'm busy today and am hoping to get a coworker to look soon (maybe tomorrow if I'm lucky). If I don't respond by Monday, please tag me on Tuesday.

@mberndt123
Copy link
Contributor Author

OK cool

@mberndt123
Copy link
Contributor Author

Ping @jsoref!

I've pushed another change to correctly handle the case where additionalFiles is empty.

@jsoref
Copy link
Collaborator

jsoref commented Jun 14, 2024

Fwiw, I've asked someone to run a test of this, so hopefully they'll check tomorrow and have feedback for me by Monday.

@mberndt123
Copy link
Contributor Author

I'm running this right now and it's working fine for me (I've added a credentials configuration for sbt and a repo configuration for scala-steward), so I'm not sure what more testing it is you need to do. But go ahead, do your thing.

@jsoref
Copy link
Collaborator

jsoref commented Jun 20, 2024

Sorry... sometimes I'm overly cautious. I'm used to things going wrong. Many things I've tried this week have gone wrong.

What I can say is that we tested the basic chart from this PR and it worked. We haven't had time to test the volume mounts although I absolutely expect them to work.

I'm going to check with a coworker and I think I'll merge by Monday.

@jsoref jsoref merged commit aeb6277 into scala-steward-org:master Jun 20, 2024
@mberndt123
Copy link
Contributor Author

mberndt123 commented Jun 21, 2024

Thanks for merging @jsoref !

@jsoref
Copy link
Collaborator

jsoref commented Jun 21, 2024

Thanks for contributing and sorry about the long time to merge.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants