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 typo in [en] resource_group_access_admin #16559

Merged
merged 10 commits into from
Jun 17, 2024
Merged

Fix typo in [en] resource_group_access_admin #16559

merged 10 commits into from
Jun 17, 2024

Conversation

Warfork
Copy link
Contributor

@Warfork Warfork commented Apr 17, 2024

What does it do?

Corrects a typo in core/lexicon/en/access.inc.php

@rthrash
Copy link
Member

rthrash commented Apr 17, 2024

This is a 100% correct copy edit fix. Good catch!

@opengeek
Copy link
Member

Thank you for your contribution! Before your pull request can be reviewed, please sign the MODX Contributor License Agreement (CLA). This is to ensure MODX has your written permission to distribute projects containing your contributions under the appropriate license.

Please make sure the CLA has been signed for GitHub user(s): @Warfork

Once you've signed, please reply with a comment letting us know so we can verify.

@Warfork
Copy link
Contributor Author

Warfork commented Apr 17, 2024

Thank you for your contribution! Before your pull request can be reviewed, please sign the MODX Contributor License Agreement (CLA). This is to ensure MODX has your written permission to distribute projects containing your contributions under the appropriate license.

Please make sure the CLA has been signed for GitHub user(s): @Warfork

Once you've signed, please reply with a comment letting us know so we can verify.

Signed.

@smg6511
Copy link
Collaborator

smg6511 commented Apr 17, 2024

@Warfork - First, it's great to see someone new contributing! Just a quick word of advice: Try to make all the changes/commits you'd foresee making before submitting the pull. Otherwise, reviewers who've already approved are going to need to come back to re-review ... and that could get a little confusing. Cheers ;-)

@smg6511
Copy link
Collaborator

smg6511 commented Apr 17, 2024

@Warfork - Also, please only make changes to the English lexicon files. All other languages are handled via Crowdin based on the current English versions.

@Warfork
Copy link
Contributor Author

Warfork commented Apr 17, 2024

@Warfork - Also, please only make changes to the English lexicon files. All other languages are handled via Crowdin based on the current English versions.

I'm really sorry to make this mistake. I will keep this in mind to prevent it from happening in the future. Do you want me to back and revert the changes to non-English lexicon files?

@Warfork
Copy link
Contributor Author

Warfork commented Apr 17, 2024

@Warfork - First, it's great to see someone new contributing! Just a quick word of advice: Try to make all the changes/commits you'd foresee making before submitting the pull. Otherwise, reviewers who've already approved are going to need to come back to re-review ... and that could get a little confusing. Cheers ;-)

Yeah this was an error on my part. You also probably want to squash everything as well. I figured there was a trust factor and a chance you might not want to accept all the corrections so I split them up.

@smg6511
Copy link
Collaborator

smg6511 commented Apr 17, 2024

so I split them up

That's definitely a solid strategy, you just want to push all those separate commits to your fork and wait until you think you're all done with the task at hand before submitting your PRs in the future.

Again, thanks for getting involved ... and ... keep it coming!

@Warfork
Copy link
Contributor Author

Warfork commented Apr 17, 2024

so I split them up

That's definitely a solid strategy, you just want to push all those separate commits to your fork and wait until you think you're all done with the task at hand before submitting your PRs in the future.

Again, thanks for getting involved ... and ... keep it coming!

Should I revert the non-English lexicon changes and make another PR here? Then hopefully you can squash the commit.

@smg6511
Copy link
Collaborator

smg6511 commented Apr 17, 2024

Should I revert the non-English lexicon changes and make another PR here?

@opengeek - Jason, would this be your preference? (@Warfork - FYI, Jason's the one who generally handles the merges.)

@Mark-H
Copy link
Collaborator

Mark-H commented Apr 17, 2024

Yes, non-English changes will need to be reverted.

@Warfork
Copy link
Contributor Author

Warfork commented Apr 17, 2024

Yes, non-English changes will need to be reverted.

Is this applicable for the files modified in setup/lang/ as well?

Edit: I took a look at crowdin and noticed the setup folder is included. Please disregard this comment.

@Warfork
Copy link
Contributor Author

Warfork commented Apr 17, 2024

Finished. After you review and squash the commits all should be well. I'm terribly sorry for over complicating something that's probably 10 lines of code in the end. Learning that you make use of crowdin as opposed to transifex was insightful.

@Warfork Warfork requested a review from Ibochkarev April 17, 2024 17:34
@opengeek
Copy link
Member

Signed.

Confirmed! Thank you for contributing!

@Warfork Warfork closed this Apr 20, 2024
@opengeek
Copy link
Member

@Warfork — why was this closed?

@Warfork
Copy link
Contributor Author

Warfork commented Apr 21, 2024

@Warfork — why was this closed?

@opengeek Sorry about that.

@Warfork Warfork reopened this Apr 21, 2024
@Warfork
Copy link
Contributor Author

Warfork commented Apr 22, 2024

@opengeek I'm not actively looking for typos, but looking over documentation to learn modx. I guess this is seemingly endless in terms of what I could find. If you want me to stop making PR's let me know. I didn't bother fixing any grammatical errors as I don't have time to contribute in that capacity. I hope this helps your team.

@smg6511 smg6511 self-requested a review April 22, 2024 13:27
@smg6511 smg6511 added this to the v3.0.6 milestone May 31, 2024
@Mark-H Mark-H added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Jun 13, 2024
@opengeek opengeek merged commit ef496e9 into modxcms:3.x Jun 17, 2024
opengeek pushed a commit that referenced this pull request Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants