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

Added a couple more notes and modifications & modified the tests to accommodate them #49

Merged
merged 8 commits into from
Oct 12, 2018

Conversation

AhmedRedaAmin
Copy link
Contributor

@AhmedRedaAmin AhmedRedaAmin commented Oct 6, 2018

In response to get-alex/alex#219

@codecov-io

This comment has been minimized.

@AhmedRedaAmin
Copy link
Contributor Author

Thanks again @jenweber for directing me to the node logs , bit of a Travis CI rookie myself unfortunately 😞

Copy link
Member

@wooorm wooorm left a comment

Choose a reason for hiding this comment

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

Hey this is very nice, thanks @AhmedRedaAmin!

One Q I commented above, and could you use lowercase for the inconsiderate patterns? The tooling matches case-insensitive, so there’s no need to use the casing here!

Let me know if you have any questions, and thanks for working on this!! 👍

@@ -436,14 +446,19 @@
- slim
inconsiderate:
- anorexic
- bony
- skinny
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, what was the reason to add skinny here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well , good question , the thing is , skinny comes from a description that someone is "skin on bones" which is used to reference people in late stages of starvation , though of course recently it has been normalized in day to day English as a casual word , however, if we are to level this description at someone who actually has anorexia for example , I thought it sounded inconsiderate in that context , or you know , I could just be overthinking this :'D.
But yeah , here is how my thought process went for both "bony" and "skinny" .
Anyways , I'll be removing skinny now .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also concerning the upper case in race.yml , I just capitalized the races ,regions , nationalities and whatnot , I wasn't aware the tooling puts everything to lower case(or uppercase) before matching , so I 'll be on it 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally , it's my pleasure , mate ! 👍

@AhmedRedaAmin
Copy link
Contributor Author

okay @wooorm , check it out and tell me what you think !

@AhmedRedaAmin
Copy link
Contributor Author

AhmedRedaAmin commented Oct 7, 2018

hey @wooorm , looks like there is a duplicate entry in the main file , it is the one causing my current merge problem

 type: simple
  source: http://ncdj.org/style-guide/
  considerate:
    - person with an intellectual disability
  inconsiderate:
    - intellectually disabled
- type: simple
  source: http://ncdj.org/style-guide/
  note: Assumes that a person with an intellectual disability has a reduced quality of life.
  considerate:
    - person with an intellectual disability
  inconsiderate:
    - suffers from intellectual disabilities
    - suffering from intellectual disabilities
    - suffering from an intellectual disability
    - afflicted with intellectual disabilities
- afflicted with a intellectual disability

Guessing from the last merge with @agans
In my version I have them both in one entry with all the variations combined + my own addition + @agans note .

@wooorm
Copy link
Member

wooorm commented Oct 8, 2018

@AhmedRedaAmin Hmm, do you see such a resolve conflicts button somewhere in the GH interface? That should be able to resolve your issue!

screenshot 2018-10-08 at 19 34 52

@AhmedRedaAmin
Copy link
Contributor Author

AhmedRedaAmin commented Oct 8, 2018

@wooorm , honestly , I thought only you had write privileges to the main repo and so I can't change it personally , however , I just noticed that I can resolve conflicts freely by altering the main repo version (not just my version) and all will be updated in the pull request . (i.e I thought the pull request had only the ability to change my fork and not the main repo )
Like , seriously , Github keeps surprising me mate :'D .
I genuinely thought in order to resolve this , I was supposed to pull the main repo again , merge it locally , alter and push (due to the divergence in commit trees) , but yeah that button was much quicker !
And thanks for letting me figure things out on my own with only minimal guidance, mate !
It's really helpful . And a sign of a good project maintainer . Thought I would mention it :'D

@AhmedRedaAmin
Copy link
Contributor Author

@wooorm could you please merge this mate ?
It is getting really frustrating having to fix merge issues the longer I wait , especially that I have done everything you asked and have no Idea why I am waiting actually , If it isn't needed , I can close the pull request though , but this weird limbo state is really triggering me , because it's showing on my home feed .

@wooorm wooorm merged commit 5c880e1 into retextjs:master Oct 12, 2018
@wooorm
Copy link
Member

wooorm commented Oct 12, 2018

thanks @AhmedRedaAmin! We appreciate it!

@wooorm wooorm added ⛵️ status/released 🗄 area/interface This affects the public interface 🦋 type/enhancement This is great to have 🧒 semver/minor This is backwards-compatible change labels Aug 13, 2019
@wooorm wooorm added the 💪 phase/solved Post is done label Jul 18, 2021
riley-martine pushed a commit to riley-martine/retext-equality that referenced this pull request Dec 7, 2023
Closes retextjsGH-49.

Reviewed-by: Titus Wormer <tituswormer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

Successfully merging this pull request may close these issues.

None yet

3 participants