Skip to content

Conversation

@martinmikula
Copy link
Contributor

I broke installation of default libraries with my prevoius commits... Now it's fixed.
BTW:
"Errors/Warnings" setting screenshots (in source code: images_plugin_dev_setup) are probably outdated checks for NLS strings are ignored (Screenshot-Preferences 1.png)
It is better to create small poll requests rather than the big one as the first was?

@jantje
Copy link
Member

jantje commented Apr 26, 2017

Seems I was adding unit testing at the same time.

It is better to create small poll requests rather than the big one as the first was?

We have discussed using gitflow (if I remember correctly) Which is creating a branch and doing your change in a branch and then "deliver" (I I'm not mistaken) I did so for a while but as there was little to no "other activity" I stopped as it is more work for me.

I think having "functional commits" is interesting but I fail quite a lot at it.
With functional commits I mean
1> adopt formatting
2> refactor XX to YY
3> add zz

@jantje jantje merged commit 02170b8 into Sloeber:master Apr 26, 2017
@rlogiacco
Copy link
Collaborator

@jantje I can't believe you found creating a branch == more work for you 😁

I suggest you stick with the practice, it really a lot cleaner to follow when you need to go back in history, which never happens until... you really need it 😀

Anyway, as you see I'm trying to find space to get back onboard 😉

@jantje
Copy link
Member

jantje commented Apr 26, 2017

unit test fails.
The servo library is not found

@martinmikula
Copy link
Contributor Author

@jantje I will work on it... Now i'm able to run unit tests... So I'm going to fix it....

@jantje
Copy link
Member

jantje commented Apr 27, 2017

txs
I changed the unit tests so they only run on 1 board instead of 3? Shall I check that in?
Also the servo library is not found in the add arduino library.
On disk I see it is installed in 2 versions which may be related

@martinmikula
Copy link
Contributor Author

martinmikula commented Apr 27, 2017

@jantje Thanks, information that library is unpacked twice is very helpfull for me.... For me the tests passed (it takes really a long time :)) It seems that this issue is something with deleting old versions, for some reason old version has not been deleted. It looks like the new version of servo lib has been added recently (because i also had two versions downloaded, but only one installed) I will continue with investigation, but it will probably takes some time, so be patient, I'm working on this project only 4th day so many things are new for me... (It takes me whole day to get unit tests properly working... :( )

Shall I check that in?

Yes, anytime, i will merge my code... :)

@jantje
Copy link
Member

jantje commented Apr 27, 2017

The 2 versions are probably caused by the method LibraryManager.installAllLatestLibraries(); Which I added a couple of days ago to do unit testing of your code.
It could be that Sloeber isn't found anymore is simply caused by the 2 versions and has nothing to do with your changes.

@jantje
Copy link
Member

jantje commented Apr 30, 2017

					if (versions != null) {
						if (versions.length == 1) {// There can only be 1
							// version of a lib
							ret.put(curLib, Lib_root.append(versions[0]));
						}

Having 2 versions causes the lib to disappear.
I'm working on it

@jantje
Copy link
Member

jantje commented Apr 30, 2017

@rlogiacco I think it is better to discuss this somewhere else but this is a quick compare to the 2 methods from my point of view
Without gitflow
After change press commit and push -> done

with gitflow
Create new branch (not that much work; but still finding a good name takes time)
After change press commit and push (same as without gitflow)
Change to Master branch
Goto browser to github and make a compare between the branch and Master to make a pull request
accept the pull request.
Pull in changes from master

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.

3 participants