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.

Sign up for GitHub

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

Jump to bottom

Implement auto redirect #10

Merged
merged 1 commit into from
Aug 30, 2020
Merged

Implement auto redirect #10

merged 1 commit into from
Aug 30, 2020

Conversation

Daltz333
Copy link
Member

@Daltz333 Daltz333 commented Aug 25, 2020 •
edited
Loading

TODO

  • When writing to file, always use forward slashes
  • When writing to file, always surround in quotes
  • Write tests

@Daltz333 Daltz333 marked this pull request as draft August 25, 2020 05:46
@Daltz333 Daltz333 marked this pull request as ready for review August 25, 2020 17:38
TheTripleV
TheTripleV requested changes Aug 25, 2020
View reviewed changes
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
sphinxext/rediraffe.py Outdated Show resolved Hide resolved
tests/test_builder.py Outdated Show resolved Hide resolved
sphinxext/rediraffe.py Outdated Show resolved Hide resolved
@TheTripleV
Copy link
Member

My PR to this branch addresses the comments to rediraffe.py.

A few upkeep things:

  1. Could you add a few more tests, covering more complex setups & rediraffe_auto_redirect_perc? The one test case was passing but was relying on an ambiguous variable.
  2. Can you further separate the checkdiff and writediff tests? (change test names or wrap in separate classes / files). I had created the tests when there was only 1 builder.

@TheTripleV
Copy link
Member

Depends on #9. This PR wraps filenames in quotes.

@TheTripleV
Copy link
Member

Closes #4.

TheTripleV
TheTripleV reviewed Aug 29, 2020
View reviewed changes
sphinxext/rediraffe.py Outdated Show resolved Hide resolved
@Daltz333 Daltz333 force-pushed the auto-file branch 6 times, most recently from ec79d23 to 6b43654 Compare August 30, 2020 03:15
@Daltz333 @TheTripleV
Implement Auto Redirect Builder
df1de60 
Co-authored-by: TheTripleV <vasistavovveti@gmail.com>
@Daltz333 Daltz333 merged commit 4026ec2 into wpilibsuite:master Aug 30, 2020
TheTripleV added a commit to TheTripleV/sphinxext-rediraffe-1 that referenced this pull request Sep 30, 2020
@Daltz333 @TheTripleV
Implement Auto Redirect Builder (wpilibsuite#10)
06b0fa0 
Co-authored-by: TheTripleV <vasistavovveti@gmail.com>

Co-authored-by: TheTripleV <vasistavovveti@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheTripleV TheTripleV TheTripleV approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Daltz333 @TheTripleV