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

Add the 'alt' attribute to the logo in the README.md #8497

Merged
merged 5 commits into from
Sep 20, 2023

Conversation

abhishekmaity
Copy link
Contributor

@abhishekmaity abhishekmaity commented Sep 17, 2023

missing 'alt' added to at README.md

@welcome
Copy link

welcome bot commented Sep 17, 2023

Yay, your first pull request towards Jenkins core was created successfully! Thank you so much!

A contributor will provide feedback soon. Meanwhile, you can join the chats and community forums to connect with other Jenkins users, developers, and maintainers.

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

Thanks!

@NotMyFault NotMyFault changed the title minor enhancement Add the 'alt' attribute to the logo in the README.md Sep 17, 2023
@NotMyFault NotMyFault added skip-changelog Should not be shown in the changelog ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback labels Sep 17, 2023
@abhishekmaity
Copy link
Contributor Author

@NotMyFault Thank you

@MarkEWaite
Copy link
Contributor

@abhishekmaity we prefer that the pull request description should match the content of the pull request and that the pull request should be submitted from a branch that is not the "master" branch. You can learn more about that process in the "Improve a plugin" tutorial.

We also prefer that the pull request checklist be completed and included in the pull request rather than being deleted. Since this pull request was a minor change to documentation, I didn't worry about requiring that you insert the full pull request template. Now that you've added significant changes and potentially incompatible changes, this pull request either needs to have those changes removed or it needs to be updated to use the pull request template

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Changes are much broader than expected for the pull request description.

I've cleared the approval from @NotMyFault because the changes require a complete new review.

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

I've cleared the approval from @NotMyFault because the changes require a complete new review.

Thanks for the PR, but as Mark mentioned, you will want to work with branches which aren't the master branch to create individual sets of PRs.

Additionally, this PR cannot be reviewed or merged in its current state. It appears you replaced all occurrences of new URL... with new URI... which doesn't work. While every URL can also be an URI, not every URI is an URL.
To address the issue, an understanding of URIs and URLs is needed.

@abhishekmaity
Copy link
Contributor Author

@NotMyFault @MarkEWaite actually I have already reverted my last commit regarding URL thing as I discovered I was going totally wrong. I therefore re-request review the change now.

@abhishekmaity
Copy link
Contributor Author

@abhishekmaity we prefer that the pull request description should match the content of the pull request and that the pull request should be submitted from a branch that is not the "master" branch. You can learn more about that process in the "Improve a plugin" tutorial.

We also prefer that the pull request checklist be completed and included in the pull request rather than being deleted. Since this pull request was a minor change to documentation, I didn't worry about requiring that you insert the full pull request template. Now that you've added significant changes and potentially incompatible changes, this pull request either needs to have those changes removed or it needs to be updated to use the pull request template

@jenkinsci/core-pr-reviewers I hope I have addressed the concerns raised by reviewers. I have already removed the significant and incompatible changes and I'm once again sorry for the mess I did to simple PR

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Thanks!

@MarkEWaite MarkEWaite added the squash-merge-me Unclean or useless commit history, should be merged only with squash-merge label Sep 19, 2023
@NotMyFault NotMyFault merged commit 7b158d2 into jenkinsci:master Sep 20, 2023
@welcome
Copy link

welcome bot commented Sep 20, 2023

Congratulations on getting your very first Jenkins core pull request merged 🎉🥳

This is a fantastic achievement, and we're thrilled to have you as part of our community! Thank you for your valuable input, and we look forward to seeing more of your contributions in the future!

We would like to invite you to join the community chats and forums to meet other Jenkins contributors 😊
Don't forget to check out the participation page to learn more about how to contribute to Jenkins.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback skip-changelog Should not be shown in the changelog squash-merge-me Unclean or useless commit history, should be merged only with squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants