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 source_location in build_vars.sh for nightly packages #7211

Merged

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Oct 28, 2021

No description provided.

@evgeni
Copy link
Member

evgeni commented Oct 28, 2021

I'd drop the quotes, they shouldn't be needed in either format (shell, key=value).

Are those files to be removed during beaching, or will they be ignored?

@ehelms
Copy link
Member Author

ehelms commented Oct 28, 2021

Are those files to be removed during beaching, or will they be ignored?

During beaching, any files present will be soaked in water and therefore have no affect on the build. There is a nightly wrapper check in the job -- https://github.com/theforeman/jenkins-jobs/pull/128/files#diff-c12500cef361f017ede0862bcabf9b799c8e4b9287be181a1d74203617208968R219

@ehelms ehelms force-pushed the add-source-location-build-vars branch from d95d852 to 5f29cf0 Compare October 28, 2021 18:08
@evgeni
Copy link
Member

evgeni commented Oct 28, 2021

🏖️ okay, yeah, I can see that, and for the non-nightly packages this is also loaded, but then we don't care about source_location, nice.

@ehelms
Copy link
Member Author

ehelms commented Oct 29, 2021

@ekohl want to give your thoughts? @evgeni are you good with the approach? The CI PR theforeman/jenkins-jobs#128 is up to date based on this PR.

@ekohl
Copy link
Member

ekohl commented Nov 1, 2021

I think this is ok. The only consideration I have is with #7202 and merge conflicts. There's probably already a few so I'd prefer to avoid rebasing all the time.

@ehelms ehelms force-pushed the add-source-location-build-vars branch from 5f29cf0 to 4bd427e Compare November 1, 2021 19:47
@ehelms
Copy link
Member Author

ehelms commented Nov 1, 2021

Updated and rebased on #7202

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I think we can try this out.

@ehelms ehelms merged commit 64f9b0f into theforeman:deb/develop Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants