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

Updated README.md #608

Conversation

FazlyMR
Copy link
Contributor

@FazlyMR FazlyMR commented Apr 5, 2022

I have tried to improve the wording for the README.md file and section of the repository.

Generally, beside improved wording, some specific changes are:

  • Encoders list in "Installation" section now split based on format/codec (AV1, VP9/VP8)
  • Added VMAF as optional supported components, including link to it's repository page.
    (Also marked as required for "Target Quality" mode. But correct me if I was wrong)

Copy link
Collaborator

@shssoichiro shssoichiro left a comment

Choose a reason for hiding this comment

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

Thanks! I think this looks fine, just a couple of cleanup items.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@redzic
Copy link
Collaborator

redzic commented Apr 6, 2022

It looks like you're requesting to merge into a branch that isn't master, and I think you have to rebase your PR onto the current master as well.

Copy link
Contributor Author

@FazlyMR FazlyMR left a comment

Choose a reason for hiding this comment

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

This is fine, thanks for the cleanup.

@FazlyMR FazlyMR changed the base branch from multiple-input-files,-sequencial-processing to master April 6, 2022 21:38
@FazlyMR FazlyMR changed the base branch from master to multiple-input-files,-sequencial-processing April 6, 2022 21:48
@FazlyMR
Copy link
Contributor Author

FazlyMR commented Apr 6, 2022

It looks like you're requesting to merge into a branch that isn't master, and I think you have to rebase your PR onto the current master as well.

I intended to contribute to the master branch, and I'm also sorry but this is my first time ever contributing to any kind of project here and don't yet know how to rebase my PR to the current master too. Any kind of guidance/help would be appreciated.

@redzic
Copy link
Collaborator

redzic commented Apr 6, 2022

I think for this PR you first need to change the base branch back to master through the github UI, and then in the terminal do something like this:

git remote add upstream https://github.com/master-of-zen/Av1an
git pull --rebase upstream master

And then push your changes back to your branch through the terminal.

Let me know if that works for you

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@FazlyMR
Copy link
Contributor Author

FazlyMR commented Apr 6, 2022

I think for this PR you first need to change the base branch back to master through the github UI, and then in the terminal do something like this:

git remote add upstream https://github.com/master-of-zen/Av1an
git pull --rebase upstream master

And then push your changes back to your branch through the terminal.

Let me know if that works for you

I've tried it but changed my mind about it since if I change the base to master then there are other files from the branch I'm committing from that are going to be pushed to the master when I don't intend to.

@redzic
Copy link
Collaborator

redzic commented Apr 6, 2022

I think you have to change the base branch, since if you leave it as-is, if we merge the PR the master branch will not actually be changed, only the multiple-input-files,-sequencial-processing branch (which is unused now) will have the updated README. Rebasing in theory should fix the problems regarding unrelated files in the PR, but let me know if you get any merge conflicts when trying the commands I posted earlier.

@FazlyMR
Copy link
Contributor Author

FazlyMR commented Apr 6, 2022

I think you have to change the base branch, since if you leave it as-is, if we merge the PR the master branch will not actually be changed, only the multiple-input-files,-sequencial-processing branch (which is unused now) will have the updated README. Rebasing in theory should fix the problems regarding unrelated files in the PR, but let me know if you get any merge conflicts when trying the commands I posted earlier.

I also never used Git before.

@redzic redzic changed the base branch from multiple-input-files,-sequencial-processing to master April 16, 2022 19:06
Co-authored-by: redzic <48274562+redzic@users.noreply.github.com>

Co-authored-by: Josh Holmer <jholmer.in@gmail.com>
@redzic redzic force-pushed the multiple-input-files,-sequencial-processing branch from 7d30366 to d7e989d Compare April 16, 2022 19:11
Copy link
Collaborator

@redzic redzic left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I went ahead and rebased your PR for you since in this case it was a bit tricky since there were merge conflicts, hopefully that is OK with you. The PR looks good now though, thanks!

@mergify mergify bot merged commit 221c56f into master-of-zen:master Apr 16, 2022
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