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

Update branching documentation after Fedora 39 branching #5089

Merged
merged 5 commits into from
Sep 4, 2023

Conversation

M4rtinK
Copy link
Contributor

@M4rtinK M4rtinK commented Aug 28, 2023

Dragging the docs back to reality - Fedora 39 edition. ;-)

The localization branch is called f<version>,
not fedora-<version>.
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Aug 28, 2023

/kickstart-test --testtype smoke

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me. However, part for the copying hashes should be fixed instead of documented.

docs/release.rst Outdated
Comment on lines 281 to 282
'fedora-38': [
'fedora-38',
],
'_manual': [
'fedora-39': [
'fedora-39',
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to adjust this each branching.

Copy link
Contributor

Choose a reason for hiding this comment

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

...fedora-NN ?

Comment on lines +317 to +312
Lastly it is necessary to set up updated l10n commit hash - check the commit hash of the ``anaconda-l10n`` repo,
the one where the new f<version> folder has been added and put the hash to the ``GIT_L10N_SHA`` variable in the
``po/l10n-config.mk`` file.

This is necessary for the Web UI related translation pinning to work & l10n branching checks to pass.

Copy link
Member

Choose a reason for hiding this comment

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

This is not necessary to be done manually. Only case is if you doing release before the anaconda-l10n automation is started. At least I would expect that.

This is definitely not something we want to do manually - copying hashes.

Copy link
Contributor

@VladimirSlavik VladimirSlavik Aug 29, 2023

Choose a reason for hiding this comment

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

If you keep the hash the same, it will fail the branching check, because the directory is literally missing. Also all subsequent rpm builds etc.... you get the idea.

To do this "automatically", run make -f Makefile.am bump-l10n-sha which will change the file. You still need to commit it manually after that.

The issue - which needs more thought, perhaps - is that the cockpit tests are not guaranteed to work after this, because they do not run on the branch itself. But maybe that's overthinking it, and making tests (and generally CI) succeed are separate steps afterwards?

Copy link
Contributor

@VladimirSlavik VladimirSlavik Aug 29, 2023

Choose a reason for hiding this comment

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

On a second thought, yes, instead of the manual steps, just run the makefile target, it will do the same.

The CI success status is a secondary concern throughout the whole process as documented, so if we want to address that, it would be another effort, probably for a less hectic branching.

docs/release.rst Outdated
Container rebuilds after branching
----------------------------------

Container rebuilds currently do not happen automatically after branching. So Do not forget to rebuild
Copy link
Member

Choose a reason for hiding this comment

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

Typo So Do not... -> So do not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. :)

Looks like the _manual bit is not there anymore.
While this is usually automated, it needs to be done manually right after
branching.
This should make it harder to forget to do that.
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Apart from the things Jirka mentioned, I see no problems with the changes.

(I might be missing something that is not changed, though. I don't remember enough to judge that.)

In the future it would be best to either automatically trigger the
rebuild or extend this section with instructions about how to
perform the procedure or at least information which containers should
be rebuilt.
@M4rtinK M4rtinK force-pushed the master-f39_fix_docs branch from eef2729 to aba6a3a Compare August 29, 2023 15:53
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Sep 4, 2023

/kickstart-test --waive webui only

@M4rtinK M4rtinK merged commit 624e48c into rhinstaller:master Sep 4, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants