Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Conversation

@tommy87
Copy link
Contributor

@tommy87 tommy87 commented May 22, 2020

Enabled the use of an apiUse block in an apiDefine block

see issue apidoc/apidoc#859

Note:
I have moved my changes to my dev branch and create a new pull request to apply only my changes. The old request was #96

var name = definition.name;
var version = block.version || packageInfos.defaultVersion;
// remove unneeded target before starting the loop, to allow a save insertion of new elements
// TODO: create a cleanup filter
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you expand on that? It's not great to add a TODO ;)

Copy link
Contributor Author

@tommy87 tommy87 May 24, 2020

Choose a reason for hiding this comment

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

This is not from me. I only moved the ToDo and the delete upwards. Before my change both were located in the foreach loop at line 127

Copy link
Contributor Author

@tommy87 tommy87 May 24, 2020

Choose a reason for hiding this comment

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

In the connected ticket you can see the changes i made in a better overview. Githubs pull request didnt resolve the extra space i add for the new while loop

Copy link
Collaborator

@NicolasCARPi NicolasCARPi left a comment

Choose a reason for hiding this comment

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

2020-05-27-141401_525x301_scrot

I'll add a .editorconfig...

@tommy87 tommy87 requested a review from NicolasCARPi May 28, 2020 13:14
@tommy87
Copy link
Contributor Author

tommy87 commented Jun 11, 2020

@NicolasCARPi
is there anything else i need todo? If not it would be great if you could merge my commits

@NicolasCARPi
Copy link
Collaborator

I'm sorry if it takes a bit of time for PRs to be merged but I need to test them first. Speaking of tests, it would be great if you could add some test for that new feature in the test suite!

@tommy87
Copy link
Contributor Author

tommy87 commented Jun 11, 2020

I can add some tests, this shoulnd be a problem. But you need to tell me where i can add the test.

@NicolasCARPi
Copy link
Collaborator

In the test folder ;)

@tommy87 tommy87 marked this pull request as draft June 13, 2020 08:56
@tommy87 tommy87 marked this pull request as ready for review June 13, 2020 13:48
@tommy87
Copy link
Contributor Author

tommy87 commented Jun 13, 2020

@NicolasCARPi
Okay i have now fixed errors shown by jshint and also add two test cases for the apiUse.

I think it would also be an big improvement for the project, if you can enable continous integration, which just run the "test" script

@tommy87 tommy87 requested a review from NicolasCARPi August 15, 2020 08:34
@tommy87
Copy link
Contributor Author

tommy87 commented Sep 23, 2020

@NicolasCARPi still waiting for the merge

If you need help with this project, i could also help as a reviewer/maintainer. At least we want to use this tool more often in our company and i really waiting for the current merge, do enable auto generation of the documentation in our CI

]
);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

this line has whitespaces (we should really have an automatic tool to look/fix these minor issues...)

@NicolasCARPi NicolasCARPi merged commit 0b075f1 into apidoc:dev Sep 23, 2020
@NicolasCARPi
Copy link
Collaborator

Hello @tommy87, you did well to remind me. Your code has been merged (sorry for the delay). I'll publish a new version soon. Ping me if not.

@NicolasCARPi
Copy link
Collaborator

If you need help with this project, i could also help as a reviewer/maintainer.

Your help would be very welcome. See with @rottmann if he's willing to give you some permissions. Even without that, please feel free to review existing PRs/issues and comment on them.

@tommy87
Copy link
Contributor Author

tommy87 commented Oct 19, 2020

Hello @tommy87, you did well to remind me. Your code has been merged (sorry for the delay). I'll publish a new version soon. Ping me if not.

Ping ;)

@tommy87
Copy link
Contributor Author

tommy87 commented Oct 19, 2020

@rottmann if you want you can add me to the list of reviewers. So i can help you a little bit

@rottmann
Copy link
Member

@tommy Welcome on board, i give you for the start "triage" rights, so you can manage issues and pull requests. Please coordinage with Nicolas, he is the "head of the department".

@NicolasCARPi Upraded you rights to maintainer. So you can manage more settings in repositories.

@NicolasCARPi
Copy link
Collaborator

@rottmann Noted!

@tommy87 Welcome! Please look at the open PRs, test the code and report if you think it's good and can be merged or if you see any issue!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants