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

B1 New Version - with Features #371

Merged
merged 19 commits into from
Jul 13, 2024
Merged

Conversation

asliprinter
Copy link
Contributor

Opening a Pull Request to Update your Project

I've started a documentation on how we should use the forking workflow here:

https://github.com/htw-imi-showtime/showtime-website/blob/documentation/doc/forking-workflow/index.md
(will not be merged to main due to file sizes)

Please follow these Steps & Guidelines for your Pull Request:

Before you Request a Review Check and Tick these Boxes:

[] make sure that the hugo site build works and check the result visually.
[] merge your local branch with upstream/main (see update doc)
[] Deploy the Site on your github pages by activating the "Deploy Page using GH-Actions" Workflow in "gh-pages-action.yml"
[] make sure that your PR contains only files / file changes within your project subdirectory.
[] make sure that all your file names consist only of lower case letters, numbers, or [-_.] - with only one . for the file extension
[] delete all unnecessary files from your PR (including cat.jpg and kitty.jpg)
[] check the image file sizes. All Images have to be as small as possible. See ./doc/disk_usage.md about this issues, there's a script: ./bin/images-smaller.sh

Note that if you fail to do so, I may simply reduce the file sizes brute force using
the script above - WITHOUT CHECKING THE QUALITY

[] if everythings seems fine, request a review by bkleinen on github. (on the upper right)

Fill out this Information:

  1. Change the PR Title to: (e.g. B0 - update techstack.md) and describe your changes:

  2. URL of your Preview Site Containing the Changes:

  3. When did your Project Supervisor review your changes? (Date/Note or require review for PR):

  4. If your disk usage exceeds 500K, provide a reason while you want to include so many images or why certain images need to be that big:

IMPORTANT: if you run into trouble / If the checks fail...

  • if you cannot figure out why a check fails or you think that it may be wrong,
    don't hesitate to contact me! (e.g. by requesting a review and commenting in the pr)

  • run the checks locally with:

    ./bin/check-project-update-pr/run-all.sh <project_id>

All good?

If all checks run successfully the PR will be reviewed and, if no
further changes are requested, merged and closed.
(this is the time to update and start a new branch!)

More Information

We are using the Forking Workflow.

We added a new subpage to for our project: features. But it was not visible on the preview on hugo. I didn't dig much deep into solving that issue because i don't want to end up causing more trouble. Prof. Kleinen, can you help us add that subpage? We also have a small gif with the size of 9mb, i hope that's fine
@bkleinen
Copy link
Contributor

again, you completely ignored upstream changes, your PR contains old commits from the first release train:
image

@bkleinen
Copy link
Contributor

Conflicting files
themes/showtime-theme-2021

-> you shouldn't change the theme. I assume you didn't, but your PR contains a change because you added the file to the commit for some reason.

@bkleinen
Copy link
Contributor

please read the new documentation in doc/forking-workflow (see above, also) to understand how to update your clone properly.

Also, as mentioned in the readme, videos are not allowed due to file size, but you can link them. Read the readme. (your gif is in fact a video).

Copy link
Contributor

@bkleinen bkleinen left a comment

Choose a reason for hiding this comment

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

Ich hatte mir schon sorgen gemacht ob meine Scripte kaputt sind, aber dieser PR geht ja von einem Stand von vor ca. 2 Wochen aus. Wenn Sie mir Zeitvorschläge machen für die nächsten 3 Tage finde ich vielleicht nochmal Zeit für ein Git 101 - bis dahin bitte mal die neue Anleitung durchlesen und PR #373 ansehen (das ist ihr branch basierend auf aktuellem main)

@asliprinter
Copy link
Contributor Author

I tried fixing the conflicts, should i send a new PR?

@bkleinen
Copy link
Contributor

bkleinen commented Jul 13, 2024

I tried fixing the conflicts, should i send a new PR?

no!

this looks much better, but one check still fails: Check Project Update PR / check for files outside of project folder (pull_request)

if you look at https://github.com/htw-imi-showtime/showtime-website/pull/371/files you'll see that your pr changes files in two other projects and the theme version.
probably the two other projects will not be happy if you change their stuff; and I'm not happy if you change the theme.

to fix the theme:

git submodule status

shows the current versions of the submodules: (if you don't have the archive, this will be different)

9a86b17ae80a5222962395d503921e4efe61e636 project-archive (heads/main)
20b1090e41e42f6cf27fcabd52f19281fb43befd themes/showtime-theme-2021 (2023.1.2-25-g20b1090)

20b109 is the most recent commit of the theme:
https://github.com/htw-imi-showtime/showtime-theme-2021/commits/main/

if yours differs:
merge in main again.
run
git submodule update

if this does not work (you might have overridden the commit sha already) go to the theme with and
pull the above version:

cd themes/showtime-theme-2021
git pull origin main

you should then see a change and commit and push this change:

cd ../..
git status

shows something like this: modified: themes/showtime-theme-2021 (new commits)

@asliprinter
Copy link
Contributor Author

Now the changes on the themes folder are gone but I couldn't solve the one with project archive

@bkleinen bkleinen merged commit c6d9075 into htw-imi-showtime:main Jul 13, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants