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 rebase script #1936

Open
wants to merge 6 commits into
base: dev-v2.6
Choose a base branch
from
Open

Conversation

aiyengar2
Copy link
Contributor

This script powered #1072, which rebased 5 charts.

Related Issue: rancher/charts-build-scripts#31

Followup to #1079.

Arvind Iyengar added 6 commits September 22, 2022 11:37
Bring in the changes in rancher#1567 that came from Jack testing the scripts out.
In newer versions of Git, these arguments cannot be provided due to git/git@9a3e3ca.

Relevant comment:

> It could be argued that this change might break existing users.  I'd argue that those existing users are already broken, and they just don't know it.  Let them know that they're broken.

# Process each commit
if [ -z "${SUB_DIRECTORY}" ]; then
COMMITS=$(git log --first-parent --oneline ${CURRENT_COMMIT}..${NEW_COMMIT} | cut -d' ' -f1 | tail -r)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest replacing ll occurances of tail -r with tac as linux GNU tail `does not support that option via the GNU docs

GNU tail can output any amount of data (some other versions of tail cannot). It also has no -r option (print in reverse), since reversing a file is really a different job from printing the end of a file; BSD tail (which is the one with -r) can only reverse files that are at most as large as its buffer, which is typically 32 KiB. A more reliable and versatile way to reverse files is the GNU tac command.

@geethub97 @aiyengar2

@nicholasSUSE
Copy link
Collaborator

Hello @aiyengar2 can I close this PR?

@joshmeranda
Copy link
Contributor

If we close this issue, I would like to have the rebase script stored somewhere outside of just the prometheus-federator. The rebase script is very helpful, especially when you have to do multiple charts / subcharts so I would like to get this into rancher/charts if possible

@recena
Copy link
Collaborator

recena commented Jan 31, 2024

@joshmeranda You can store the script in your forked repository. In order to improve the traceability and the automation, we plan to set one PR → one chart (modify).

@nicholasSUSE
Copy link
Collaborator

@joshmeranda You can store the script in your forked repository. In order to improve the traceability and the automation, we plan to set one PR → one chart (modify).

This seems promising @recena I will keep this one open.
Once the efforts for the 2.7.11 and 2.8.1 sync are finished me and @lucasmlp will evaluate it.

@joshmeranda
Copy link
Contributor

Ended up writing a replacement tool for this script to get around some of the pain points with the script. If @rancher/mapps is interested, it would be trivial to integrate it into charts-build-scripts.

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.

6 participants