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

Fix typos on docs #594

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Fix typos on docs #594

merged 1 commit into from
Apr 23, 2024

Conversation

LavredisG
Copy link
Contributor

@LavredisG LavredisG commented Apr 21, 2024

DCO keeps failing despite the sign-off being included in the commit message.

Fix: after removing privacy option from github email, git cache had to be updated to reflect the changes.

@karmada-bot karmada-bot added the kind/documentation Categorizes issue or PR as related to documentation. label Apr 21, 2024
@karmada-bot karmada-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 21, 2024
@RainbowMango
Copy link
Member

DCO keeps failing despite the sign-off being included in the commit message

cc @zhzhuang-zju to take a look at the DCO.

/assign @windsonsea
I'm not sure if we need a blank here.

@zhzhuang-zju
Copy link
Contributor

DCO keeps failing despite the sign-off being included in the commit message

@LavredisG The error mssg of DCO is 'Commit sha: 92f02a2, Author: LavredisG, Committer: GitHub; Expected "LavredisG 90918919+LavredisG@users.noreply.github.com", but got "LavredisG lavredisgoume@gmail.com".', which looks like a mailbox problem

@RainbowMango
Copy link
Member

How to fix it?

@zhzhuang-zju
Copy link
Contributor

How to fix it?

The DCO check should be fine, so @LavredisG need to set github settings related to email address, which can be found at https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/adding-an-email-address-to-your-github-account

@windsonsea
Copy link
Member

Normally we only need to commit again like:

git commit -s --amend
git push -f

If it doesn't work, you may need to follow #594 (comment)

@LavredisG
Copy link
Contributor Author

DCO keeps failing despite the sign-off being included in the commit message

@LavredisG The error mssg of DCO is 'Commit sha: 92f02a2, Author: LavredisG, Committer: GitHub; Expected "LavredisG 90918919+LavredisG@users.noreply.github.com", but got "LavredisG lavredisgoume@gmail.com".', which looks like a mailbox problem

Thanks a lot! I never noticed it.

@zhzhuang-zju
Copy link
Contributor

Hi @LavredisG Looks like the DCO failed for the same reason. Have you tried the methods mentioned in #594 (comment) and #594 (comment)? Looking forward to your feedback~

@LavredisG
Copy link
Contributor Author

LavredisG commented Apr 22, 2024

Hi @LavredisG Looks like the DCO failed for the same reason. Have you tried the methods mentioned in #594 (comment) and #594 (comment)? Looking forward to your feedback~

I changed the email settings, the issue was due to the fact that I had enabled privacy email. My guess is that DCO has kept the mail that initiated the PR as the one it expects, so I will close this and retry once I am available.


- Support 1:n mapping of policy: workload. Users don't need to indicate scheduling constraints every time creating federated applications.
- Support 1:n mapping of policy:workload. Users don't need to indicate scheduling constraints every time creating federated applications.
Copy link
Contributor

Choose a reason for hiding this comment

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

@LavredisG Thanks for your work! Curious to ask:

  • What are the norms for whether to add spaces before and after punctuation marks in English documents?
  • Did you scan these "problem" with a tool?
  • I observed that the website repo has a ci for checking typo, does it cover these "problems"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Regarding parentheses and brackets, "a space should be added before the opening parenthesis or bracket, but not before the closing parenthesis or bracket".
  • No, I found them while reading the docs for my understading.
  • I hadn't check the ci, it seems that it checks for typos rather than correct punctuation. Also, words such as "Karmada" aren't typical English words, so the Karamda typo would go unchecked.


- Support 1:n mapping of policy: workload. Users don't need to indicate scheduling constraints every time creating federated applications.
- Support 1:n mapping of policy:workload. Users don't need to indicate scheduling constraints every time creating federated applications.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Support 1:n mapping of policy:workload. Users don't need to indicate scheduling constraints every time creating federated applications.
- Support 1:n mapping of policy:workloads. Users don't need to indicate scheduling constraints every time creating federated applications.

How about appending the suffix s, so policy:workloads has a consistent pattern with 1:n?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 nice finding

Signed-off-by: LavredisG <lavredisgoume@gmail.com>
@LavredisG LavredisG changed the title Update concepts.md Fix typos on docs Apr 22, 2024
@zhzhuang-zju
Copy link
Contributor

@LavredisG Glad to see you solved the DCO issue. Would you be able to tell how you solved the problem? That way if other contributors run into the same problem in the future, they'll have a good example of how to fix it!

Copy link
Member

@windsonsea windsonsea left a comment

Choose a reason for hiding this comment

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

/lgtm

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 23, 2024
@LavredisG
Copy link
Contributor Author

@LavredisG Glad to see you solved the DCO issue. Would you be able to tell how you solved the problem? That way if other contributors run into the same problem in the future, they'll have a good example of how to fix it!

Sure, I have already written it in the PR's description, but I will add it here for readability too:

"Error: DCO keeps failing despite the sign-off being included in the commit message.

Fix: after removing privacy option from github email, git cache (not github) had to be updated to reflect the changes."

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/approve

@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 23, 2024
@karmada-bot karmada-bot merged commit d4ac31e into karmada-io:main Apr 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants