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

Issue# 430, Fixing Indentation levels #464

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

zevnux
Copy link

@zevnux zevnux commented Dec 2, 2015

Fixed indentation level detection. Made the following changes.

  • Added error 117 and 118 to detect misuse of tabs or spaces depending on the initial tab
  • Added new variable initial_tab to hold and remember the initial tab type used
  • Changed tab detection from 8 spaces to "\t" for consistency

@zevnux zevnux closed this Dec 2, 2015
@zevnux zevnux reopened this Dec 2, 2015
@zevnux
Copy link
Author

zevnux commented Dec 2, 2015

Fixed most errors, some errors seem to be due to changes to detecting the indentation.

@sigmavirus24
Copy link
Member

A few questions:

  1. Why did you commit .DSStore? Could you remove this please?
  2. Why did you create a whole new directory of tests when there already exists a directory of tests? Could you consolidate them?
  3. Why did you push so much unnecessary history? Could you rebase a bunch of these commits out?
  4. Why did you choose a bug to fix that had not been previously triaged by the maintainers?

@zevnux zevnux force-pushed the master branch 2 times, most recently from f670376 to 49623d5 Compare December 2, 2015 18:29
@zevnux
Copy link
Author

zevnux commented Dec 2, 2015

  1. This is my bad, I've removed it
  2. We separated our tests out initially, they have now been moved to the E11.py test file
  3. I wasn't aware of the rebase functionality, I've tried rebasing it but not sure if it's at the correct commit right now.

Forgive us for our lack of knowledge, we're all very new with contributing to open source projects

and 4. This was for a university project, with submitting a pull request being part of the requirement.

As there are still some issues, it may be better to hold off on this pull until they're all taken into account. Maybe creating a new issue with specific requirements will help figure out what needs to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants