Skip to content

Conversation

@doms99
Copy link

@doms99 doms99 commented Nov 14, 2024

Added ability to add automatic changelog generation.

Projects should add new .changelog-generator.sh file that defines generate_changelog function that will be an entry point to the generation flow.

Added an example from NutriU (made by @jabou) to generate changelog on projects that use JIRA.

@doms99 doms99 requested a review from jabou November 14, 2024 15:06
@jabou
Copy link
Member

jabou commented Feb 6, 2025

@doms99 can you please update your branch from the base as we made some major changes to the base branch that could impact your implementation.

Thank you! :)

@doms99 doms99 force-pushed the feature/changelog-generator-setup branch 2 times, most recently from 5da13a6 to 188491f Compare February 11, 2025 11:54
@doms99 doms99 force-pushed the feature/changelog-generator-setup branch from 188491f to 3cbf480 Compare February 11, 2025 13:12
@doms99 doms99 marked this pull request as ready for review February 12, 2025 09:24
@doms99
Copy link
Author

doms99 commented Feb 12, 2025

@jabou I made required adjustments, let me know how it looks to you

Copy link
Member

@jabou jabou left a comment

Choose a reason for hiding this comment

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

Nice work! :)

Let's discuss some comments I left in here before we proceed. Once the main implementation is defined and approved, we should consider migration logic or something like that as hitting init will probably override deploy-options.sh we don't want to change.

Comment on lines +28 to +29
# Set to "" if you don't want to use this feature.
BRANCH_NAME_TASK_MATCHING_REGEX="(<project>-[0-9]+)"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Set to "" if you don't want to use this feature.
BRANCH_NAME_TASK_MATCHING_REGEX="(<project>-[0-9]+)"
# Set to BRANCH_NAME_TASK_MATCHING_REGEX to "" if you don't want to use this feature.
BRANCH_NAME_TASK_MATCHING_REGEX="(<project>-[0-9]+)"

It should be self-explanatory, but on first reading, I thought that I had to set to ""


# JIRA project URL. Used by python script to construct a url to specific issue.
# Replace <project> with your JIRA project name.
JIRA_PROJECT_URL="https://<project>.atlassian.net"
Copy link
Member

Choose a reason for hiding this comment

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

Let's create a few additional variables at the top of the file where the user will have to write a value for this <project>, and BRANCH_NAME_TASK_MATCHING_REGEX's <project>.

It's always better to write those bash scripts in a way that you have some "global" config variables that the user needs to define/fill (even better, some yaml file, but let's not complicate for now), instead of the user needing to read through the file and search for all templates they have to fill/change.

echo "Missing JIRA_EMAIL or JIRA_TOKEN. Please add it to your .zshrc or .bash_profile configuration file."
echo

return
Copy link
Member

Choose a reason for hiding this comment

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

Should we call exit or return some non 0 code so that program stops with the execution?

Comment on lines +50 to +51
# Task number contained in branch name
current_branch=`git rev-parse --abbrev-ref HEAD`
Copy link
Member

Choose a reason for hiding this comment

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

I think the main script already has this info. Please check and use that global var instead of rereading it with git command.

# Task number contained in branch name
current_branch=`git rev-parse --abbrev-ref HEAD`
if [[ $current_branch =~ $BRANCH_NAME_TASK_MATCHING_REGEX ]]; then
task_numbers=$(printf "%s " "${BASH_REMATCH[@]}")
Copy link
Member

Choose a reason for hiding this comment

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

Be careful with printf. Not all bash versions support it. I think I had issues with it, so I tried to avoid using it.

echo "Enter task number contained in this build, separated by space (e.g., <project>-1234 <project>-5678)."
echo "For manual changelog entry, leave input empty and press enter."
echo
read -r -p "Tasks contained in this build (e.g. <project>-1234 <project>-5678): " task_numbers
Copy link
Member

Choose a reason for hiding this comment

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

If we have already defined <project> within a global variable for this script, is it necessary for the user to retype it?
Can we maybe ask the user only for the task number (i.e., 1234, 5678) and then construct tasks using <project> from various + entered values?

fi

# Select or edit changelog, edit tasks list and generate again
while true; do
Copy link
Member

Choose a reason for hiding this comment

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

🤔 not sure I would use this approach as this could easily introduce an infinite loop...

execution will be paused when we call for a text editor. Maybe we can just leverage that? E.g., check how creating a regular tag opens the text editor, and while the editor is open, the script waits for input.

break
fi

__call_python_script "$task_numbers"
Copy link
Member

Choose a reason for hiding this comment

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

Be sure that whatever is behind __call_python_script, is supported on all Bash versions. Check which version came with macOS (at least 2-3 versions in the past).

Copy link
Member

Choose a reason for hiding this comment

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

I know using Python would ease the implementation; however, I would consider using pure bash as much as possible, as with this one, we have now introduced a new dependency and requirement that the user needs to have Python 3.8.

Comment on lines +31 to 33
if [[ ${c} =~ ^(yes|y|Y) ]] || [ -z ${c} ]; then
__init_deploy_options
fi
Copy link
Member

Choose a reason for hiding this comment

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

If the user enters N or just hits enter, what would happen if this logic is removed? Will execution stop?

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