Project

General

Profile

Bug #10507

flake cleanup of generate_sssd_conf.py

Added by Josh Paetzel about 5 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Nice to have
Assignee:
John Hixson
Category:
OS
Target version:
Severity:
New
Reason for Closing:
Reason for Blocked:
Needs QA:
Yes
Needs Doc:
Yes
Needs Merging:
Yes
Needs Automation:
No
Support Suite Ticket:
n/a
Hardware Configuration:
ChangeLog Required:
No

Description

Need a sign off from John before merging.

There's one change, the addition of lines = []
that I'm not sure about.

Associated revisions

Revision 87e2dbdf (diff)
Added by Josh Paetzel about 5 years ago

flake cleanup There are a change I am unsure about. Do not merge until john has been consulted. Ticket: #10507 Merge-FN93: Merge-TN93:

Revision 0f2af877 (diff)
Added by John Hixson about 5 years ago

Screwed by py-flakes Ticket: #10507

Revision c3305d9d (diff)
Added by Josh Paetzel about 5 years ago

flake cleanup There are a change I am unsure about. Do not merge until john has been consulted. Ticket: #10507 Merge-FN93: Merge-TN93: (cherry picked from commit 87e2dbdfa8b37ad7dbedd9f903c9ceaa80f8cabd)

Revision e1d1d0d4 (diff)
Added by John Hixson about 5 years ago

Screwed by py-flakes Ticket: #10507 (cherry picked from commit 0f2af877823230e168d96e68569ca60a0ad71ec0)

Revision 60cb94fa (diff)
Added by Josh Paetzel about 5 years ago

flake cleanup There are a change I am unsure about. Do not merge until john has been consulted. Ticket: #10507 Merge-FN93: Merge-TN93: (cherry picked from commit 87e2dbdfa8b37ad7dbedd9f903c9ceaa80f8cabd)

Revision 66622260 (diff)
Added by John Hixson about 5 years ago

Screwed by py-flakes Ticket: #10507 (cherry picked from commit 0f2af877823230e168d96e68569ca60a0ad71ec0)

History

#1 Updated by John Hixson about 5 years ago

  • Status changed from Unscreened to Investigation

Things look okay, however, I do recall python being a little picky about "is not" vs "!= ", so I'm going to need to test this a bit before merging.

#2 Updated by John Hixson about 5 years ago

So yeah, flakes flagged and moved aux_sc out of scope which causes it to be referenced before it's assigned. I don't find that very helpful ;-) It's supposed to clean up code, not break it ;-) More to come!

#3 Updated by Jordan Hubbard about 5 years ago

  • Priority changed from No priority to Nice to have

#4 Updated by Josh Paetzel about 5 years ago

It's just a linting tool. Without comments or documentation or unit tests it's hard to unwind a nearly 1000 line file, and I totally believe the linting tool did the wrong thing.

Which is why I passed it over to you. I knew it would break something. ;)

#5 Updated by Josh Paetzel about 5 years ago

As far as that particular change, that is the danger with using tabs for indentation. The linter replaced the tab with 4 spaces, when it was an 8 space tab.

#6 Updated by John Hixson about 5 years ago

Josh Paetzel wrote:

As far as that particular change, that is the danger with using tabs for indentation. The linter replaced the tab with 4 spaces, when it was an 8 space tab.

No tabs used, just spaces.

#7 Updated by John Hixson about 5 years ago

John Hixson wrote:

Josh Paetzel wrote:

As far as that particular change, that is the danger with using tabs for indentation. The linter replaced the tab with 4 spaces, when it was an 8 space tab.

No tabs used, just spaces.

Okay. I was wrong. I checked an old copy and sure enough it was tabbed. I honestly have no idea why I did that, habit perhaps? Either way, I still think pyflakes should have done the right thing ;-)

#8 Updated by Josh Paetzel about 5 years ago

It couldn't.

Since on your system a tab was defined as 4 spaces and it was 8 on mine. The poor tool never had a chance.

This is why mixing tabs and spaces is a bad idea on a system where indentation matters. If a script is all tabbed or all spaced it is portable between systems, if it mixes tabs and spaces it's interpretation becomes dependent on the definition of a tab.

In this case your system and mine disagreed and chaos was outcome.

#9 Updated by John Hixson about 5 years ago

Josh Paetzel wrote:

It couldn't.

Since on your system a tab was defined as 4 spaces and it was 8 on mine. The poor tool never had a chance.

This is why mixing tabs and spaces is a bad idea on a system where indentation matters. If a script is all tabbed or all spaced it is portable between systems, if it mixes tabs and spaces it's interpretation becomes dependent on the definition of a tab.

In this case your system and mine disagreed and chaos was outcome.

IMO, if it doesn't know what to do with it, because of exactly the situation you described, it shouldn't do anything at all and just dump a warning to the screen ;-) My $0.02

#10 Updated by Josh Paetzel about 5 years ago

That is exactly what it did, then a human (myself) with no docs or comments or specs and a different tab stop than you made a decision.

The important part here was that to me, the indentation didn't change (visually).

Depending on how a tabstop is defined changes how the file looks when the other indentation is spaces.

Neither the tool nor I had a chance. Now a reasonable position to that is "then don't change a thing" but the linter was reporting dozens of errors, which made it very difficult to validate that the bug fix I was doing was correct.

#11 Updated by John Hixson about 5 years ago

  • Status changed from Investigation to Ready For Release

I did not flake on flake cleanup with flakes.

#12 Updated by Jordan Hubbard about 5 years ago

  • Status changed from Ready For Release to Resolved

#13 Avatar?id=14398&size=24x24 Updated by Kris Moore about 4 years ago

  • Target version changed from Unspecified to N/A

Also available in: Atom PDF