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

removed sdg-img-item class #4518

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

kurikurichan
Copy link
Member

Fixes #4422

What changes did you make and why did you make them ?

  • Removed lines 263-268 in the _project-page.scss file
  • That particular SCSS class is not being used anywhere in the site, which is why we removed it
  • My "good first issue"

@cmedina-dev cmedina-dev self-requested a review April 14, 2023 23:28
@cmedina-dev
Copy link
Contributor

Review ETA: 9:00 PM CST 4/15/23
Availability: Weekdays after 6:00 PM CST, Weekends flexible

Copy link
Contributor

@cmedina-dev cmedina-dev left a comment

Choose a reason for hiding this comment

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

Hi Krista! I read over your PR. Everything looks good! You made sure to link the issue and that the change originates from the feature branch, removed the requested lines of code, and stated the reason for the change. Only thing I might suggest in the future is avoiding the extra white space changes as they tend to clutter the PR. Other than that small suggestion, great job!

@drakenguyen4000 drakenguyen4000 self-requested a review April 15, 2023 03:51
@drakenguyen4000
Copy link
Member

ETA 4/14/23
Availability: M-F 3:30pm-8pm PST

Copy link
Member

@drakenguyen4000 drakenguyen4000 left a comment

Choose a reason for hiding this comment

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

Hi @kurikurichan, great job listing the changes you made and why you did them. The PR is linked to its issue; however, please don't forget to move your PR to the Project Board.

Commit change approved! Nicely done!

@roslynwythe
Copy link
Member

Hi @kurikurichan please move the PR to the Project Board, and we will get it merged. Thank you !

@kurikurichan
Copy link
Member Author

Thank you!! I will do that now

@Adastros
Copy link
Member

Hi @kurikurichan, one last thing! Could you add the same labels in issue #4422 on this pull request? The labels are usually automatically added by the github actions bot. It wasn't added this time since this pull request wasn't added to the project board on creation.

@chrismenke45 chrismenke45 merged commit 3e80825 into hackforla:gh-pages Apr 19, 2023
@kurikurichan kurikurichan added good first issue Good for newcomers role: front end Tasks for front end developers Feature: Refactor CSS Page is working fine - CSS needs changes to become consistent with other pages size: 0.25pt Can be done in 0.5 to 1.5 hours labels Apr 19, 2023
@kurikurichan
Copy link
Member Author

Thanks @Adastros, I did that just now. For future reference, is there a way I should add the pull request to the project board on creation?

@kurikurichan kurikurichan deleted the remove-sdg-class-4422 branch April 19, 2023 23:33
@Adastros
Copy link
Member

Hi @kurikurichan, thanks for adding the labels! You can add the pull request before submitting it by clicking the gear icon as shown below.

image

Previously I mentioned adding the pull request to the project board before submitting but you can do it after or before. Thinking about it more, the labels probably weren't added automatically due to some changes to the project board that occurred around the time you submitted the pull request. Those changes caused some issues with the bots. You can check the details for this merge and see one of the errors for this pull request.

It also helps to double check the labels just in case the bots didn't add them.

Reference this section of contributing.md for adding the pull request to the project board.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Refactor CSS Page is working fine - CSS needs changes to become consistent with other pages good first issue Good for newcomers role: front end Tasks for front end developers size: 0.25pt Can be done in 0.5 to 1.5 hours
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove .sdg-img-item class from _project-page.scss
6 participants