-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
pylint-ci branch committed to but never merged #365
Comments
It looks to me like the If this is the case then I think we go with option (3) - replacing |
This exact issue just came up during a workshop, so I think that should definitely be the case |
Okay, I think I have tracked down the source of the problem. It is the introduction of the @sjvrijn did you or your instructors not see that section? It is understandable because it is buried at the bottom of quite a long lesson. We could perhaps consider turning it into its own episode so that it is harder for learners to miss. For example, if you decide as a class to skip the content on using Pylint, then it would be easy to miss those steps. I will wait for your feedback on that, and we can consider the move to a separate episode based on that. |
@bielsnohr It's that exact section that causes the confusion, since it explicitly instructs to merge |
At the end of the 'Diagnosing Issues and Improving Performance' episode, the final git instructions lead to a confusing situation, where a new
pylint-ci
branch is created fromdevelop
and committed to, but is never merged intodevelop
ortest-suite
. This causes confusion in later episodes when the CI does not work as expected.Git history at this point in the episode
Condensed instructions from the last two sections in the episode:
Git history at the end of the episode:
I see a couple of options, but what would the preferred order of operations be here?
pylint-ci
intodevelop
, separately from mergingtest-suite
intodevelop
pylint-ci
intotest-suite
before the merge intodevelop
pylint-ci
branch and simply commit totest-suite
insteadThe text was updated successfully, but these errors were encountered: