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

Write default attr values #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Write default attr values #2

wants to merge 2 commits into from

Conversation

HEnquist
Copy link
Member

If a controller is deployed with sardanadsconfig the default values of attributes are not written to the database. Normally this is done by the defctrl or defelem macros. Depending on the implementation of an affected controller this may or may not be a problem, but it can lead to strange and annoying errors. This PR adds a check when the Pool starts to write any missing default value after each device is started.

@amilan
Copy link

amilan commented Nov 13, 2019

Hi @HEnquist, thanks for the PR!
I think it's a nice addition.
I can only suggest a couple of things. Instead of merging this branch to the master branch, it's better if the merge points to the develop branch, because the master branch is only used for the creations of new releases and it stays stable.
The develop branch is the one used to add new features that will be included in the next release.

The second thing depends on how we want to proceed, because we have two options: doing the PR against the develop branch in our fork and after that, create a new PR to the upstream repository, or skip this step in between and do the PR directly to the develop branch in the upstream repository.

I think I prefer the second option because it remove one step in between, but let me know your opinion and we can do one or the other.

Cheers,
Antonio.

@HEnquist
Copy link
Member Author

Hi @amilan and thanks for reviewing!
I agree with you that the second option is better. The only reason I pointed it to master was that I had some trouble getting develop to run in my environment, and I was lazy so I just went with master instead. I'll rebase this on develop. Should I then make a PR to the official sardana repo?
Cheers,
Henrik

@amilan
Copy link

amilan commented Nov 15, 2019

Hi again,

It will be great if you could make the PR directly to the upstream repo, but if you feel more comfortable doing it here, I can try to redirect it later.

Anyway, thanks again for the good job!!

@HEnquist
Copy link
Member Author

Done! New PR here: sardana-org#1236

HEnquist pushed a commit that referenced this pull request Nov 11, 2020
Fix compatibility with python 2.6
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