Skip to content

Update developer setup doc to use scripts in the virtualenv - #4808

Closed
Matt Boersma (mboersma) wants to merge 1 commit into
Azure:devfrom
mboersma:update-dev-docs
Closed

Update developer setup doc to use scripts in the virtualenv#4808
Matt Boersma (mboersma) wants to merge 1 commit into
Azure:devfrom
mboersma:update-dev-docs

Conversation

@mboersma

Copy link
Copy Markdown
Member

Since the wrapper scripts under tools/scripts were removed in b5e53a8 and not replaced, I'm guessing we should call the python module entrypoints directly. This updates the "configuring your machine" developer doc to do that.

Closes #4806.

@troydai

Copy link
Copy Markdown
Contributor

The check_style and run_test command will always available if dev_setup.py was run. They should be the permanent entry points. Thank you for helping with the document.

@mboersma

Copy link
Copy Markdown
Member Author

Ah, is that how it works now? My bad--I'll change this to use:

./env/bin/check_style
./env/bin/run_tests

and so on.

@troydai

Copy link
Copy Markdown
Contributor

Here's how it works (I should have documented it. This is on me.)

During the dev_setup.py, the tools folder is installed as a package (see here). As a result the scripts defined in the tools package (see here) are added to the current virtualenv.

Hence directly referencing is discouraged.

@mboersma Matt Boersma (mboersma) changed the title Update developer setup doc to use automation entrypoints directly Update developer setup doc to use scripts in the virtualenv Nov 2, 2017
@mboersma

Matt Boersma (mboersma) commented Nov 2, 2017

Copy link
Copy Markdown
Member Author

During the dev_setup.py, the tools folder is installed as a package

Ah I see--that's nifty. Troy Dai (@troydai) I updated this to refer to those scripts.

@troydai

Copy link
Copy Markdown
Contributor

Willie Xu (@williexu) just updated the doc. Thank you. #4844

@mboersma

Matt Boersma (mboersma) commented Nov 7, 2017

Copy link
Copy Markdown
Member Author

Some advice: it's always going to be preferable to work with an existing PR from a contributor instead of just fix it yourself.

This was not a lot of work, but I did notice the problem first (as I did the previous several times you broke this doc), and I fixed it. After several days with no feedback, my PR was replaced by a similar one which was immediately approved and merged. Every step in that process sent me a clear message that my contribution was unwanted.

My main interest is in keeping the developer documentation accurate, and I'm glad it is fixed now after almost a week. I'm a Microsoft employee who is just trying to help az out when I notice problems, but I strongly encourage you to imagine how this would look to an outside contributor.

@troydai

Copy link
Copy Markdown
Contributor

Hi Matt Boersma (@mboersma) , I apologize that if the way in which we handle this leaves the impression that your effort is not appreciated. I can assure you that your effort is very appreciated and you're the first one to bring our attention to the problem of our documentation. It is very helpful.

What happened is that over the weekend developers from two different sources urged us to fix this documentation quickly. One is a developer in PR #4804 and the other sent us emails about the progress of the status.

I notice this PR hasn't been moving for a few days and I wondered if you're busy with the feature. Instead of pushing you on this issue, I asked Willie Xu (@williexu) to make a change in separate PR.

In the hindsight, I should have communicated with you before we move on our own. We will do better next time.

Again, we do appreciate your contribution to the features as well as the documentation. And I'm sorry for any misunderstanding.

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