search

JMSLab / xtevent

Stata package -xtevent-
MIT License
43 stars 12 forks source link

#152 Helpfile clarifications #158

Closed jorpppp closed 11 months ago

jorpppp commented 1 year ago

Closes #152

jorpppp commented 1 year ago

@Constantino-Carreto-Romero Can you take a look at this to see that everything is in order before merging, please?

Constantino-Carreto-Romero commented 1 year ago

@jorpppp I already checked and left some comments

jorpppp commented 11 months ago

@Constantino-Carreto-Romero I agree with your suggestions from your review about the overidoptions. Can you review and make sure that the helpfile here and the helpfile in EventStudyPackage coincide? That may mean some edits both here and in the EventStudyPackage repo.

I am not sure what your suggestion is for the redundant imputation Case. Did we clarify this already in eventstudypackage?

Constantino-Carreto-Romero commented 11 months ago

@jorpppp

@Constantino-Carreto-Romero I agree with your suggestions from your review about the overidoptions.

Should I implement the changes in my review above?

Can you review and make sure that the helpfile here and the helpfile in EventStudyPackage coincide? That may mean some edits both here and in the EventStudyPackage repo.

I've found some slight differences between the help file in the paper draft and the help file here. The paper draft is more updated. I think I should first propagate the changes from my review above to the paper draft, then propagate the recent edits in the paper draft to the help file here.

I am not sure what your suggestion is for the redundant imputation Case. Did we clarify this already in eventstudypackage?

We already edited it in the paper draft as in the attached image. I think that description is clearer than the one here in the help file. Should I change the impute description here in accordance to the description in the paper draft?

1

jorpppp commented 11 months ago

@jorpppp

@Constantino-Carreto-Romero I agree with your suggestions from your review about the overidoptions.

Should I implement the changes in my review above?

Can you review and make sure that the helpfile here and the helpfile in EventStudyPackage coincide? That may mean some edits both here and in the EventStudyPackage repo.

I've found some slight differences between the help file in the paper draft and the help file here. The paper draft is more updated. I think I should first propagate the changes from my review above to the paper draft, then propagate the recent edits in the paper draft to the help file here.

I am not sure what your suggestion is for the redundant imputation Case. Did we clarify this already in eventstudypackage?

We already edited it in the paper draft as in the attached image. I think that description is clearer than the one here in the help file. Should I change the impute description here in accordance to the description in the paper draft?

1

Agreed, thank you @Constantino-Carreto-Romero. Let's proceed as you suggest then: Let's propagate these minor changes to the paper, then bring the paper's helpfile here.

@jmshapir FYI

jorpppp commented 11 months ago

Are we ready to merge @Constantino-Carreto-Romero ?

Constantino-Carreto-Romero commented 11 months ago

@jorpppp