grambank / pygrambank

Apache License 2.0
4 stars 1 forks source link

Adding a line to make api ignore other files besides #14

Closed HedvigS closed 4 years ago

HedvigS commented 4 years ago

I'd like for api.py to not just ignore 'gitattributes' when it passes things onto sheet.py but also other files like the ".DS_store" that Mac os creates. I think this change accomplishes this.

HedvigS commented 4 years ago

I'm not sure what I did last time, but right now I can't run pygrambank check until somehow at least ".DS_store" is excluded from what api.py sends to sheet.py. If there is another solution, I'm all ears.

HedvigS commented 4 years ago

Simon's change request seemed good, I can't approve it because I'm not a reviewer (as far as I understand things).

HedvigS commented 4 years ago

I don't know what it means that this PR failed "all checks", but I'm choosing to not worry about it if @xrotwang isn't worried.

xrotwang commented 4 years ago

I think files in this directory other than sheets and .gitattributes is an error which should be corrected in the repos rather than ignoring it.

Hedvig Skirgård notifications@github.com schrieb am Fr., 10. Juli 2020, 12:31:

I don't know what it means that this PR failed "all checks", but I'm choosing to not worry about it if @xrotwang https://github.com/xrotwang isn't worried.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/glottobank/pygrambank/pull/14#issuecomment-656606327, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGUOKCNZ66JY5DTNYZHWHLR23UZHANCNFSM4OWOBZDA .

HedvigS commented 4 years ago

@xrotwang Ok. So what should I do about things like ".DS_store" which mac os constantly recreates?

HedvigS commented 4 years ago

I'm happy to do this some other way. This to me seemed like the easiest way to fix the problem I am having, but if there are others I'm all ears. Until I have some way of bypassing files like ".DS_Store", I can't run this command.

HedvigS commented 4 years ago

Are we alright to merge this now? I don't fully understand the UI surrounding "requested changes"

xrotwang commented 4 years ago

@SimonGreenhill ok to merge now and think about more flexibility later?

SimonGreenhill commented 4 years ago

sounds good.