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

update sumolib. with Condition and earlyTarget in Phase #15230

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

aminissn
Copy link
Contributor

I added my python code under tools/tls with all necessary changes in sumolib init

@namdre
Copy link
Contributor

namdre commented Jul 17, 2024

  • Please put busPriority.py into a separate pull request (it's not quite ready for inclusion, while the rest of the code is fine)
  • Please add the new vTypes argument in induction_loop.py at the end of the argument list to ensure backward compatibility with existing code (default argument should be the empty string).

@aminissn
Copy link
Contributor Author

hopefully this is fine, even though the failing eca test bother me every time!

@behrisch
Copy link
Contributor

Sorry about the ECA thing, I reopened the issue at Eclipse. Could you try to do this without f-strings? Unfortunately there are still some python2 users out there.

@autumnfound
Copy link

hopefully this is fine, even though the failing eca test bother me every time!

@aminissn I missed it due to a red herring last time, but we found the issue now! You have 2 Eclipse accounts, and the one that's linked to this GH account doesn't have an ECA, while the one that has an ECA doesn't have the Github account link. We'll need to at least clear the GH field of the unused account if you want to update the account you've been using, but the real solution is fixing the accounts and merging them into one. Can you check in on https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/4780#note_2547110 so we can get that started for you?

@autumnfound
Copy link

Also to clarify, I can confirm at this time that the email you used in association with the commits DOES have an ECA, but when we do Github checks, we use the Github username when available as it's an explicit link to accounts. That explains why when we check your email its fine, and why your account has an ECA, but fails here.

@aminissn
Copy link
Contributor Author

aminissn commented Jul 18, 2024

Also to clarify, I can confirm at this time that the email you used in association with the commits DOES have an ECA, but when we do Github checks, we use the Github username when available as it's an explicit link to accounts. That explains why when we check your email its fine, and why your account has an ECA, but fails here.

Is there any specific solution for this? I am using the same email address for both Github and Eclipse.
UPDATE: I removed my Github username from my other Eclipse account and added it to this email address. can I revoke this pull request or shall I create a new one to test if it works?

@aminissn aminissn changed the title added busPriority.py update sumolib. with Condition and earlyTarget in Phase Jul 18, 2024
@aminissn
Copy link
Contributor Author

can we merge these changes now? so I can keep working from the latest development version of the main distribution for bus priority?

@m-kro
Copy link
Contributor

m-kro commented Jul 24, 2024

As @behrisch wrote, please avoid the f-strings for compatibility reasons. Something more:

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.

5 participants