Skip to content

Conversation

@Kami
Copy link
Contributor

@Kami Kami commented Feb 17, 2020

I tried to get COMP_EXECUTABLE setting to work with the following values, but It didn't appear to work:

COMP_EXECUTABLES = [
    ('Python 2.7.17 - idle conf 1', 'L'),
    ('Python 3.5.9 - idle conf 1', 'L'),
    ('Python 2.7.17 - loaded conf 1', 'L'),
    ('Python 3.5.9 - loaded conf 2', 'L'),
]

After digging into the code, I noticed that the code hard codes the default branch value name to default.

This obviously won't work when a default branch is not called default. I assume this default value is a carry-over / legacy from Mercurial support (please correct me if I'm missing or misunderstanding something)...


In this pull request, I updated it to use default_branch value of the Project model.

I believe this is a correct change and I tested it with a project where value of default_branch is performance_tracking and it works correctly.

With this change, name in the left "Executables" select box menu will now also be correct and won't include ... in branch ... part when a default branch is used.

Here is an example with the same COMP_EXECUTABLES setting value and Project.default_branch being set to performance_tracking.

Before:

Screenshot from 2020-02-17 16-27-45

After:

Screenshot from 2020-02-17 16-32-02

TODO

  • Tests

Kami added 2 commits February 17, 2020 16:28
branch being called "default" instead of reading the value of
default_branch attribute of the project model.

With this change / fix COMP_EXECUTABLES now works correctly when a
default branch is called something else than "default".
handles custom (aka non "default") default project branches.
@Kami
Copy link
Contributor Author

Kami commented Feb 17, 2020

I've added a test case in 393d0cd.

Copy link
Owner

@tobami tobami left a comment

Choose a reason for hiding this comment

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

👍 Looks like we'll have to do a release soon!

@Kami
Copy link
Contributor Author

Kami commented Feb 19, 2020

Pushed a fix for a build failure due to a bad merge conflict resolution I did through the Github WebUI.

@Kami
Copy link
Contributor Author

Kami commented Feb 24, 2020

@tobami Anything else I need to do here?

If not, I appreciate if you can merge it when you get a chance so I can work on tests for #283 (those PRs touch the same files so I will wait until this PR is merged to avoid conflicts).

Thanks.

@tobami
Copy link
Owner

tobami commented Feb 24, 2020

@tobami Anything else I need to do here?

nope, it's a very necessary fix, thanks a lot. Merging...

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.

2 participants