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

New artwork for Open-Shell #1226

Merged
merged 2 commits into from
Dec 7, 2022
Merged

New artwork for Open-Shell #1226

merged 2 commits into from
Dec 7, 2022

Conversation

bonzibudd
Copy link
Member

This adds the icon and additional artwork as discussed in #31.

@AppVeyorBot
Copy link

@ge0rdi
Copy link
Member

ge0rdi commented Dec 6, 2022

Looks good to me.

Just the rounded corner on install screen looks a bit odd (well, I'm not designer):
image

Win10/11 doesn't seem to use much rounded corners.

@ge0rdi ge0rdi self-requested a review December 6, 2022 07:31
@ge0rdi
Copy link
Member

ge0rdi commented Dec 6, 2022

One technical note.

Commits Improve setup banner icon and Fix web.ico size are basically fixups of earlier New icons and installer artwork (they just fix things introduced in that original commit).
It will be nice to squash them into original commit in the end.

You can do that easily using git rebase -i origin/master (you can search for git interactive rebase if you are not familiar with it).

@bonzibudd
Copy link
Member Author

I see what you mean with the rounded corners, I will adjust that. I will combine the fix commits as well.

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@bonzibudd
Copy link
Member Author

@ge0rdi I made the changes, but I don't think I should have updated from open-shell-master as they are showing as new commits now. Let me know if I need to change anything from my end.

@ge0rdi
Copy link
Member

ge0rdi commented Dec 6, 2022

Hmm, you should sync your master with Open-Shell repo.

First, make sure you have upstream set up, using this guide:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/configuring-a-remote-repository-for-a-fork

Then you should do:
git fetch upstream
git rebase upstream/master

This should end up with just your 2 commits on your master.
Then use git push -f to push that to your repo.

@ge0rdi
Copy link
Member

ge0rdi commented Dec 6, 2022

It looks good now.

But AppVeyor is ignoring those changes because:
Commit "e310e60a" of branch "master" skipped as commit files did not match "only_commits.files"

I think it's because the last commit on your branch modifies just readme.md and we are not triggering builds on such changes.

Try to swap those commits (using git rebase -i origin/master).
So that the last commit on branch will change files in Src folder.

@bonzibudd
Copy link
Member Author

bonzibudd commented Dec 6, 2022

Ok, I think it's fixed now. I'll do some spot-checking, but I think it's good to go.

@AppVeyorBot
Copy link

@bonzibudd
Copy link
Member Author

Update: I found a possible issue with the installer art.

@ge0rdi I'd like to wait on pushing this for the moment.

This adds the new artwork for the main icon and installer screens.
Addresses #31.

Update classic button thumbnail

Improve setup banner icon

Fix web.ico size

New installer art

Fix dialog2
@AppVeyorBot
Copy link

@bonzibudd
Copy link
Member Author

@ge0rdi

Alright, everything looks fine now. I had made a silly mistake in the resolution of Dialog2.jpg but it should be correct now.

Copy link
Member

@ge0rdi ge0rdi 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 👍

@ge0rdi ge0rdi merged commit d28c8e5 into Open-Shell:master Dec 7, 2022
@bonzibudd
Copy link
Member Author

@ge0rdi this is off-topic, but I am planning on making a couple changes to Readme. Is it possible to make one PR for both readme and gh-pages, or will they need to be separate?

@ge0rdi
Copy link
Member

ge0rdi commented Dec 7, 2022

Nope.
They are on different branches so you need two PRs.
It should be possible to do edits via web interface and it will automatically create new branch and PR from it.

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.

None yet

3 participants