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 ask for tags script #340

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

valentin-krasontovitsch
Copy link
Contributor

The idea of this script is to automate part of the release management process - it allows checking for internal dependencies that have changes on their default branch since their latest release, and composes a message that can be used to poke maintainers and invite them to create a new tag.

@valentin-krasontovitsch valentin-krasontovitsch force-pushed the ask-for-tags-message-script branch 2 times, most recently from f1f4b6a to 42569f5 Compare February 10, 2023 12:39
The idea of this script is to automate part of the release management
process - it allows checking for internal dependencies that have changes
on their default branch since their latest release, and composes a
message that can be used to poke maintainers and invite them to create a
new tag.
@oysteoh
Copy link
Contributor

oysteoh commented Feb 13, 2023

As discussed Valentin - this PR needs to wait until the release process and proposal of changing it has been approved in Scout

Comment on lines +22 to +23
bleeding_release_file = "releases/matrices/bleeding.yml"
duplicate_pkgs = ["everest-models"] # this is included in the everest repo
Copy link
Member

Choose a reason for hiding this comment

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

In principle, komodo is a general purpose FOSS tool. I wonder if it's possible to write the functions in this module in such a way that they do not assume it's only being run by Equinor? I will mark some other lines that can perhaps be refactored.

Copy link
Member

Choose a reason for hiding this comment

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

(Just to add: another possibility is that this project is not meant for general consumption at all, which is totally fine -- but then we should either close it or make it clear that it's an Equinor tool.)

Comment on lines +43 to +44
DEFAULT_FORK = "equinor"
ALTERNATE_FORK = "tno-everest"
Copy link
Member

Choose a reason for hiding this comment

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

More Equinor-specific stuff.

Comment on lines +94 to +97
def get_scout_maintainers(gh_client: Github) -> Mapping[str, str]:
scout_repo = gh_client.get_repo("equinor/scout")
projects_file = scout_repo.get_contents("projects.md")
projects_file_contents = base64.b64decode(projects_file.content).decode()
Copy link
Member

Choose a reason for hiding this comment

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

More Equinor specific stuff. Maybe we can generalize this, I'm not sure. We've been wondering if other parts of the code could benefit from a local config file -- possibly this is locations.yaml or maybe some new thing. This seems like the kind of thing that could go in there.

raise ValueError(
"Please provide a github access token through the env var `GITHUB_TOKEN`\n"
"Note that the token should be a personal access token "
"(not fine-grained),\n configured for SSO with equinor"
Copy link
Member

Choose a reason for hiding this comment

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

More Equinor-specific stuff.

@valentin-krasontovitsch
Copy link
Contributor Author

thanks for the comments @kwinkunks - i was thinking along those same lines, and of course you are right, this violates 12-factor in so many ways : )

i also now discovered a bug, and will try to address the points you raise while working on a fix!

@oysteoh
Copy link
Contributor

oysteoh commented Feb 13, 2023

thanks for the comments @kwinkunks - i was thinking along those same lines, and of course you are right, this violates 12-factor in so many ways : )

i also now discovered a bug, and will try to address the points you raise while working on a fix!

I would also suggest to wait with the changes and pause work on this PR until we have a decision on how we want to proceed with the release process.

@valentin-krasontovitsch valentin-krasontovitsch removed their assignment Aug 29, 2023
@valentin-krasontovitsch
Copy link
Contributor Author

i'm unassigning myself as I'm leaving scout - @eivindjahren, since we talked a lot about this, maybe you could try to find somebody to take this further (e.g. current up and coming release manager @yngve-sk), or close as not desired

@jonathan-eq
Copy link
Collaborator

@eivindjahren Do you think we should close this as abandoned?

@eivindjahren
Copy link
Contributor

@jonathan-eq I think this is still up for discussion,

@eivindjahren eivindjahren added the christmas-review Issues and PRs for Christmas review label Dec 13, 2024
@jonathan-eq jonathan-eq removed the christmas-review Issues and PRs for Christmas review label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

5 participants